Re: [Simple] memory leak in ReportMechanism.messageCounters

Joao Antunes <joao.antunes@tagus.ist.utl.pt> Wed, 29 January 2014 14:26 UTC

Return-Path: <joao.antunes@tagus.ist.utl.pt>
X-Original-To: simple@ietfa.amsl.com
Delivered-To: simple@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 71ED11A0218 for <simple@ietfa.amsl.com>; Wed, 29 Jan 2014 06:26:28 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.813
X-Spam-Level:
X-Spam-Status: No, score=-1.813 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, FM_FORGED_GMAIL=0.622, HTML_MESSAGE=0.001, RP_MATCHES_RCVD=-0.535, SPF_HELO_PASS=-0.001] autolearn=ham
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id utbRMfDcWGgF for <simple@ietfa.amsl.com>; Wed, 29 Jan 2014 06:26:24 -0800 (PST)
Received: from smtp2.ist.utl.pt (smtp2.ist.utl.pt [IPv6:2001:690:2100:1::16]) by ietfa.amsl.com (Postfix) with ESMTP id C4ED41A03CB for <simple@ietf.org>; Wed, 29 Jan 2014 06:26:23 -0800 (PST)
Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp2.ist.utl.pt (Postfix) with ESMTP id A7659700036F for <simple@ietf.org>; Wed, 29 Jan 2014 14:26:19 +0000 (WET)
X-Virus-Scanned: by amavisd-new-2.6.4 (20090625) (Debian) at ist.utl.pt
Received: from smtp2.ist.utl.pt ([127.0.0.1]) by localhost (smtp2.ist.utl.pt [127.0.0.1]) (amavisd-new, port 10025) with LMTP id 5b3udq0+Y3Mp for <simple@ietf.org>; Wed, 29 Jan 2014 14:26:19 +0000 (WET)
Received: from mail2.ist.utl.pt (mail.ist.utl.pt [IPv6:2001:690:2100:1::8]) by smtp2.ist.utl.pt (Postfix) with ESMTP id 1C79D70003B7 for <simple@ietf.org>; Wed, 29 Jan 2014 14:26:19 +0000 (WET)
Received: from mail-oa0-f48.google.com (mail-oa0-f48.google.com [209.85.219.48]) (Authenticated sender: ist154457) by mail2.ist.utl.pt (Postfix) with ESMTPSA id A839620021DB for <simple@ietf.org>; Wed, 29 Jan 2014 14:26:18 +0000 (WET)
Received: by mail-oa0-f48.google.com with SMTP id l6so2052139oag.7 for <simple@ietf.org>; Wed, 29 Jan 2014 06:26:17 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=mime-version:reply-to:in-reply-to:references:date:message-id :subject:from:to:content-type; bh=EDhn5VMCRdfCGH4t0silSdxHMWN8ZDUpBx/7HxCo3fI=; b=lM8smjN0q/Mow9QJ3Ewdnrf+/KIBF9X6cjLQlbKWIiSYkUvo9cYuRMKx3QeH+2iBJU M9iEWpCMZY48oG1o97HTBehjwDgoMLJQM49FxtQHfu8IjuJ70qI9gIY2q6qkby0TWyj1 DE6Iy4XUDTKh4k47dRyq3k+LUyNBHkdhGGnmFBqpaEWoPvb8tqArvxb2tbliNUcKOGA3 6409UDLhoNfTHCeQY00KaPB83s06s3scMaiJKllTUeEHaCruQbXdiZCTjG5KmuYLHtXu QyEvZCsENyQym1WjrnrWclESv3L6cjLCEkMzkDg6Xm3wVb+cwiPoVupTlNJD2BqdPter X81g==
MIME-Version: 1.0
X-Received: by 10.60.94.71 with SMTP id da7mr343624oeb.83.1391005577500; Wed, 29 Jan 2014 06:26:17 -0800 (PST)
Received: by 10.76.180.74 with HTTP; Wed, 29 Jan 2014 06:26:17 -0800 (PST)
In-Reply-To: <E6C23A45F92B3B4B801E95713764578C461906AF@isrtlvexmx4.interwise.com>
References: <E6C23A45F92B3B4B801E95713764578C461906AF@isrtlvexmx4.interwise.com>
Date: Wed, 29 Jan 2014 15:26:17 +0100
Message-ID: <CAOK9a7SKNZT9KDtY9=cPoFgWaOzetiWVsR5u-ZB_5q69UQeLMA@mail.gmail.com>
From: Joao Antunes <joao.antunes@tagus.ist.utl.pt>
To: issues@msrp.java.net, "dev@msrp.java.net" <dev@msrp.java.net>, simple@ietf.org
Content-Type: multipart/alternative; boundary=089e01228aea53effa04f11cb9f2
Subject: Re: [Simple] memory leak in ReportMechanism.messageCounters
X-BeenThere: simple@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
Reply-To: joao.antunes@tagus.ist.utl.pt
List-Id: SIP for Instant Messaging and Presence Leveraging Extensions <simple.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/simple>, <mailto:simple-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/simple/>
List-Post: <mailto:simple@ietf.org>
List-Help: <mailto:simple-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/simple>, <mailto:simple-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 29 Jan 2014 14:26:28 -0000

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 <rkadison@interwise.com>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>