Re: [reSIProcate] Memory leak when creating the SDP from string

[Diego Carvalho Domingos <ddomingos@xxxxxxxxxx>]

Hi Scott, thanks for answering. The problem seems to be when hfv is passed to Contents. Pidf was just an example. I’m using SdpContents. But both use the same structure for their constructors:

Pidf::Pidf(const HeaderFieldValue& hfv, const Mime& contentsType)

   : Contents(hfv, contentsType),

     mNote()

{

}

SdpContents::SdpContents(const HeaderFieldValue& hfv, const Mime& contentTypes)

   : Contents(hfv, contentTypes)

{

}

The problem is that the Contents constructor being called is:

 

Contents::Contents(const HeaderFieldValue& headerFieldValue,

                   const Mime& contentType)

   : LazyParser(headerFieldValue),

      mType(contentType)

{

   init();

}

And not:

Contents::Contents(const HeaderFieldValue& headerFieldValue,

                     HeaderFieldValue::CopyPaddingEnum e,

                     const Mime& contentsType)

    : LazyParser(headerFieldValue,e),

    mType(contentsType)

{

   init();

}

If the second one was being used, the copied hvf would have the ownership.
So, my function is pretty similar to that of the link in my previous message. I’ll copy over:

 

SdpContents* generateBody()

       {

          Data* txt = new Data("v=0\r\n"

                               "o=1900 369696545 369696545 IN IP4 192.168.2.15\r\n"

                               "s=X-Lite\r\n"

                               "c=IN IP4 192.168.2.15\r\n"

                               "t=0 0\r\n"

                               "m=audio 8000 RTP/AVP 8 3 101\r\n"

                               "a=rtpmap:8 pcma/8000\r\n"

                               "a=rtpmap:3 gsm/8000\r\n"

                               "a=rtpmap:101 telephone-event/8000\r\n"

                               "a=fmtp:101 0-15\r\n");

         

          HeaderFieldValue hfv(txt->data(), txt->size());

          SdpContents *sdp = new SdpContents(hfv, Mime("application", "sdp"));

          return sdp;

       }

 

When “new SdpContents(hfv, Mime("application", "sdp"))” is called, that copy I explained above is performed. If I use the init method you suggested, it will delete the data when the local hfv goes out of scope and will crash since the ownership was not passed to SdpContents’ hfv. Note sure if this is a design problem or I’m missing something here. The only solution I see is to pass the data along and defer the creation of SdpContents to the moment it is used. Let me know if you have another idea. Thanks.

 

Diego

 

From: Scott Godin <sgodin@xxxxxxxxxxxxxxx>
Sent: Monday, November 12, 2018 12:46 PM
To: Diego Carvalho Domingos <ddomingos@xxxxxxxxxx>
Cc: resiprocate-devel@xxxxxxxxxxxxxxx
Subject: Re: [reSIProcate] Memory leak when creating the SDP from string

 

Hi Diego,

 

Many resip classes will try to use memory as efficiently as possible, by avoiding data copies.  If the a SIP message block received off the wire is stored in it's entirety, then the objects that attach to it to provide parsing, endeavor to avoid creating copies of all the header bytes, or body byte as they are parsed.   So what is expected in your examples is the the original Data block you provided stays around for as long as the resulting Pidf contents class.  If the Data goes away and the Pidf class sticks around longer, then it will core.

 

In your case, you really want HeaderFieldValue to take ownership of the Data or to copy it and own it.  There is no interface to have HeaderFieldValue take ownership of a Data object, so....

 

Looking at the HeaderFieldValue class, seems like the easiest thing to use would be the init method, which allows you to specify if it should own the data or not.  I haven't tested this, but give it a go....

  Data text(publishBody);

   HeaderFieldValue hfv;

   hfv.init(text.data(), text.size(), true /* own? */);

   Mime type("application", "pidf+xml");

   Pidf pc(hfv, type);

   boost::shared_ptr<resip::Contents> body(&pc);

 

Scott

 

 

 

 

 

On Fri, Nov 9, 2018 at 1:31 PM Diego Carvalho Domingos <ddomingos@xxxxxxxxxx> wrote:

Recently I found that my code has a leak when creating the SDP from string. I tried to find the correct way of doing this in examples/tests but could not understand who is responsible to delete the Data pointer. For instance, I found this:

/*

   WHAT PREVIOUSLY WORKED BUT NOW CORES...

   Data text(publishBody);

   HeaderFieldValue hfv(text.data(), text.size());

   Mime type("application", "pidf+xml");

   Pidf pc(&hfv, type);

   boost::shared_ptr<resip::Contents> body(&pc);

  

   ....

   WHAT NOW NEEDS TO BE DONE INSTEAD..

   */

   Data* text = new Data(publishBody);

   HeaderFieldValue hfv(text->data(), text->size());

   Mime type("application", "pidf+xml");

   Pidf* pc = new Pidf(hfv, type);

   boost::shared_ptr<resip::Pidf> body(pc);

   /*

   */

Is this the correct way? If so, who deletes the data in this code? I could not find in either HeaderFieldValue (in the constructor that takes a char * and size, mMine is set to false) nor in Contents (in my case SdpContents).
Another example, which is very similar to my case, is the generateBody in this example (I believe it is no longer in the current code base):
http://svn.resiprocate.org/dox/dum/testSMIMEInvite_8cxx_source.html#l00232
I did not test but I guess there is a leak in this example as well. Who deletes the pointer returned by generateBody and who deletes the Data * created inside the method?

I appreciate any help. Thanks in advance. Att,

Diego Domingos

_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel