Re: [Simple] memory leak in ReportMechanism.messageCounters

[Joao Antunes <[e-mail]>]

Hi Reuven!

It was a lifetime ago that I wrote that code, and I wasn't very mindful of
the possible leaks. So it might happen. However, I didn't used the code
that much to get any kind of idea if there is a leak.

I can tell you what that class is supposed to do and when the
messageCounters should be discarded.

Basicly, MSRP allows for chunks of a Message to be sent over the 'wire',
therefore, the chunks can be fragmented and arrive in a different sequence
from the one they were sent. E.g. with a message that has 1024 bytes, that
MSRP finds fit (I don't remember exactly how he decides/negotiates how big
the chunks will be, but that's besides the point anyway) to divide in
chunks of 128 bytes = 8 chunks. The job of the Counter is to keep track of
the chunks it already receives, so that the receive endpoint can send
accurate REPORT messages back to the other endpoint when requested.

I guess the perfect moment to clear those counters is when a session is
teared down, and all of the Counters associated with the Messages that
belong to that Session (now I'm not sure anymore if Messages must belong to
a session, or if the same message id can be used across sessions [more
unlikely]). But it seems that the Messages do belong to a Session (not a
connection, a session can have multiple connections if i'm not mistaken),
according to this fragment from the RFC:

"It is possible that an endpoint will receive a REPORT request on a
   session that is no longer valid.  The endpoint's behavior if this
   happens is a matter of local policy.  The endpoint is not required to
   take any steps to facilitate such late delivery; i.e., it is not
   expected to keep a connection active in case late REPORTs might
   arrive."

from: http://tools.ietf.org/html/rfc4975#page-27

It has been a lifetime ago, so I added the MSRP mailing list to this
discussion so that they can correct me if I'm wrong.

If you are willing to change the code so that it behaves like that, we
would welcome very much your patch so that it could be added to the
codebase.

Another optimization that can be done, is that the counter discards the
array of bytes or bits that it uses, and replaces it with a simple boolean
when all of the message is received. That ought to save memory untill the
Session is teared down.

If you have more questions, please contact us.

Cheers,
João Antunes




On Mon, Jan 27, 2014 at 8:59 AM, Reuven Kadison <[e-mail]>wrote:

>  Hi,
>
>
>
> I just got a memory leak and found out that DefaultReportMechanism has a
> member messageCounters (defined in ReportMechanism) that is never cleared.
>
> Can someone please check this?
>
>
>
> Regards,
>
> *Reuven Kadison.*
>
>
>
>
>
>
>
> ************************************************************************************
> This footnote confirms that this email message has been scanned by
> PineApp Mail-SeCure for the presence of malicious code, vandals & computer
> viruses.
>
> ************************************************************************************
>
>


-- 
João Antunes
http://web.ist.utl.pt/~joao.a.p.antunes/<http://web.ist.utl.pt/~joao.a.p.antunes/?reference=emailSig>