Re: memory leak in ReportMechanism.messageCounters

--===============0484177617064178023==
Content-Type: multipart/alternative; boundary=089e01228aea53effa04f11cb9f2

--089e01228aea53effa04f11cb9f2
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

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 =3D 8 chunks. The job of the Counter is to keep track o=
f
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=E3o Antunes




On Mon, Jan 27, 2014 at 8:59 AM, Reuven Kadison <[email protected]>wro=
te:

>  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 & compute=
r
> viruses.
>
> *************************************************************************=
***********
>
>


--=20
Jo=E3o Antunes
http://web.ist.utl.pt/~joao.a.p.antunes/<http://web.ist.utl.pt/~joao.a.p.an=
tunes/?reference=3DemailSig>

--089e01228aea53effa04f11cb9f2
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Hi Reuven!<div><br></div><div>It was a lifetime ago that I=
 wrote that code, and I wasn&#39;t very mindful of the possible leaks. So i=
t might happen. However, I didn&#39;t used the code that much to get any ki=
nd of idea if there is a leak.</div>
<div><br></div><div>I can tell you what that class is supposed to do and wh=
en the messageCounters should be discarded.</div><div><br></div><div>Basicl=
y, MSRP allows for chunks of a Message to be sent over the &#39;wire&#39;, =
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&#39;t remember exactly how he decides/negotiates how =
big the chunks will be, but that&#39;s besides the point anyway) to divide =
in chunks of 128 bytes =3D 8 chunks. The job of the Counter is to keep trac=
k of the chunks it already receives, so that the receive endpoint can send =
accurate REPORT messages back to the other endpoint when requested.</div>
<div><br></div><div>I guess the perfect moment to clear those counters is w=
hen a session is teared down, and all of the Counters associated with the M=
essages that belong to that Session (now I&#39;m not sure anymore if Messag=
es must belong to a session, or if the same message id can be used across s=
essions [more unlikely]). But it seems that the Messages do belong to a Ses=
sion (not a connection, a session can have multiple connections if i&#39;m =
not mistaken), according to this fragment from the RFC:</div>
<div><br></div>&quot;It is possible that an endpoint will receive a REPORT =
request on a<br>=A0 =A0session that is no longer valid. =A0The endpoint&#39=
;s behavior if this<br>=A0 =A0happens is a matter of local policy. =A0The e=
ndpoint is not required to<br>
=A0 =A0take any steps to facilitate such late delivery; i.e., it is not<br>=
=A0 =A0expected to keep a connection active in case late REPORTs might<br>=
=A0 =A0arrive.&quot;<div><br></div><div>from: <a href=3D"http://tools.ietf.=
org/html/rfc4975#page-27">http://tools.ietf.org/html/rfc4975#page-27</a><br=
>
</div><div><br></div><div>It has been a lifetime ago, so I added the MSRP m=
ailing list to this discussion so that they can correct me if I&#39;m wrong=
.=A0</div><div><br></div><div>If you are willing to change the code so that=
 it behaves like that, we would welcome very much your patch so that it cou=
ld be added to the codebase.</div>
<div><br></div><div>Another optimization that can be done, is that the coun=
ter 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 me=
mory untill the Session is teared down.</div>
<div><br></div><div>If you have more questions, please contact us.</div><di=
v><br></div><div>Cheers,</div><div>Jo=E3o Antunes</div><div><br></div><div>=
<br></div></div><div class=3D"gmail_extra"><br><br><div class=3D"gmail_quot=
e">
On Mon, Jan 27, 2014 at 8:59 AM, Reuven Kadison <span dir=3D"ltr">&lt;<a hr=
ef=3D"mailto:[email protected]" target=3D"_blank">[email protected]=
om</a>&gt;</span> wrote:<br><blockquote class=3D"gmail_quote" style=3D"marg=
in:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">






<div lang=3D"EN-US" link=3D"blue" vlink=3D"purple">
<div>
<p class=3D"MsoNormal">Hi,<u></u><u></u></p>
<p class=3D"MsoNormal"><u></u>=A0<u></u></p>
<p class=3D"MsoNormal">I just got a memory leak and found out that DefaultR=
eportMechanism has a member messageCounters (defined in ReportMechanism) th=
at is never cleared.<u></u><u></u></p>
<p class=3D"MsoNormal">Can someone please check this?<u></u><u></u></p>
<p class=3D"MsoNormal"><u></u>=A0<u></u></p>
<p class=3D"MsoNormal">Regards<span style=3D"font-size:14.0pt;font-family:&=
quot;Monotype Corsiva&quot;;color:#1f497d">,<u></u><u></u></span></p>
<p class=3D"MsoNormal"><b><span style=3D"font-size:14.0pt;font-family:&quot=
;Monotype Corsiva&quot;;color:#1f497d">Reuven Kadison.</span></b><b><span s=
tyle=3D"font-size:10.0pt;font-family:&quot;Verdana&quot;,&quot;sans-serif&q=
uot;;color:#1f497d"><u></u><u></u></span></b></p>

<p class=3D"MsoNormal"><u></u>=A0<u></u></p>
</div>
<br><br>
 <br> <br>*****************************************************************=
*******************<br>This footnote confirms that this email message has b=
een scanned by<br>PineApp Mail-SeCure for the presence of malicious code, v=
andals &amp; computer viruses.<br>
***************************************************************************=
*********<br><br>
</div>



</blockquote></div><br><br clear=3D"all"><div><br></div>-- <br>Jo=E3o Antun=
es<div><a href=3D"http://web.ist.utl.pt/~joao.a.p.antunes/?reference=3Demai=
lSig" target=3D"_blank">http://web.ist.utl.pt/~joao.a.p.antunes/</a></div>
</div>

--089e01228aea53effa04f11cb9f2--

--===============0484177617064178023==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Simple mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/simple

--===============0484177617064178023==--