Memory leaked per connection

[Daniel Nurmi]

It looks like the rampart_context structure is not freed after processing (in rampart_in_handler.c), and in the rampart_context_free function, the section for freeing the receiver_cert is commented out (in rampart_token_processor.c). In our application (eucalyptus), this is causing the back-end components (axis2c/rampartc web services) to leak memory on every connection, which adds up quickly under load (30-50MB/day). I've attached a patch for review, but since the free section was commented out, it may be that there are other rampart configurations that will fail using this patch (this problem could certainly use some upstream guidance!)

== SRU Report ==

Impact: A small amount of memory is not freed for every connection to an Eucalyptus web service. Under load, this can add up to 50Mb of RAM per day.

Fix in development branch: This needs to be fixed in two places: Eucalyptus and Rampart. The Eucalyptus patch is applied to 1.6.1 in Lucid. The Rampart fix has been shipped as 1.3.0-0ubuntu7, same patch applied.

Minimal patches:
Eucalyptus: http://bazaar.launchpad.net/~eucalyptus-maintainers/eucalyptus/1.6/revision/946
Rampart: debian/patches/rampart-memleak.patch (see comment 18 below)

TEST CASE:
Run a UEC CC/NC setup under load and watch the memory footprint of the associated apache2 processes. Without the patch, the memory used grows, with the patch it should remain stable.

Regression potential: The patch affects memory management of Rampart structures and therefore has some potential for regression. However it was tested and accepted upstream (comment 7) and tested on real setups at Eucalyptus Systems. The only package in archive that uses rampart is eucalyptus. The regression tests are therefore quite limited.

[Thierry Carrez (ttx)]

Not a bug in eucalyptus

[Thierry Carrez (ttx)]

About "other rampart configurations", we added rampart for eucalyptus so if the patch is needed to work properly with the way eucalyptus uses rampart, I'd say it takes precedence over other unknown ways that may rely on the buggy behavior.

But knowing *why* this was commented out would certainly help in the discussion.

[Thierry Carrez (ttx)]

Asked upstream, since they explicitly commented this out in a "memory leak fix" revision:
http://svn.apache.org/viewvc?view=revision&revision=709349

[Daniel Nurmi (nurmi)]

the patch required a small patch to eucalyptus that initializes axis2c in the CC for each NC client connection (most already were handled this way, except to StartNetwork/RunInstances).

in r946

[Thierry Carrez (ttx)]

Rampart upstream is looking at the patch.

[Thierry Carrez (ttx)]

Comments on the patch from Rampart upstream (shankar):

The patch is having several problems. First of all, if the
rampart_context is freed in in_handler, server side functionalities
will be broken. rampart_context is actually freed when msg_ctx is
freed. So no need to free it.

Uncommenting the lines to free receiver_cert is fine, but it causes
crash in some scenarios due to a bug. I fixed it in trunk. If you are
working on 1.3.0 release, I have attached a patch. You have to apply
it as well to fix the crash.

[Daniel Nurmi (nurmi)]

Thierry, Shankar,

Thank you both for following up on this issue and finding a potential patch to the memory leak issue in rampart; I'm testing the rampartc-1.3.0 with the above patch now with a long term eucalyptus test, and will report back when the test completes (we usually run for .5 day to make sure that memory is not creeping up). So far, it looks great.

[Thierry Carrez (ttx)]

Dan: thanks for testing this.

Did you simply apply both patches (yours and shankar's), or did you also remove the rampart_context_free in in_handler from your patch ? In doubt, could you attach the combined patch (against rampart 1.3.0) that you are currently testing ?

[Thierry Carrez (ttx)]

@Dan:
Any news about how did your long term test go ?
Please also answer to question in comment 9, so that I know what you exactly tested :)

[Thierry Carrez (ttx)]

Eucalyptus part of the fix now committed to lp:~ubuntu-core-dev/eucalyptus/ubuntu-karmic

[Launchpad Janitor (janitor)]

This bug was fixed in the package eucalyptus - 1.6.1~bzr1083-0ubuntu1

---------------
eucalyptus (1.6.1~bzr1083-0ubuntu1) lucid; urgency=low

  [ Dustin Kirkland ]
  * Merge upstream bzr revision 1082; the following bugs have been fixed
    upstream since the last merge:
    - LP: #378969 - private bug
    - LP: #404842 - init script fix
    - LP: #434283 - existing keys should be overwritten unconditionally
    - LP: #445990 - run instance will fail if no kernel or ramdisk specified
    - LP: #447457 - euca_conf --register-sc ... check the number of parameters
    - LP: #449874 - fix incorrect help text (--delete-nodes doesn't exist)
    - LP: #451795 - show registered images in elastic fox
    - LP: #454405 - return correct networkIndex values on describeInstances
    - LP: #456877 - init script fix
    - LP: #456878 - fix for libvirt xen driver
    - LP: #460085 - fix rampart memory leak
    - LP: #461156 - fix authentication problem w/ userdata
    - LP: #461394 - fix multiple concurrent snapshots on the same volume
    - LP: #461444 - fix memory leaks in NC getConsoleOutput and startup_thread
    - LP: #469984 - fix iptables rules issue
    - LP: #477776 - fix query string authentication
    - LP: #480783 - allow api connection over https
    - LP: #482249 - fix "Describe Regions"
    - LP: #484217 - create keypair should return an error if key exists
    - LP: #490623 - parse RFC 1123 formatted datetime
  * debian/control:
    - make all package lists one-per-line (makes changes henceforth more
      readable), sort lists
    - depend on rampart >= 1.3.0-0ubuntu6, which fixes some shared library
      installation issues
  * debian/patches/04-axis2c-1.6.0-rampart-1.3.0.patch: drop this patch,
    since Eucalyptus 1.6.1 natively supports axis2c 1.6.0 now
  * debian/eucalyptus-cloud.install,
    debian/eucalyptus-common.eucalyptus.upstart,
    debian/eucalyptus-java-common.install, debian/eucalyptus-sc.install,
    debian/eucalyptus-walrus.install: update static version number strings
    from "1.6-devel" to "1.6.1"; (we should really find a better way to do
    this)
  * debian/patches/03-DESTDIR.patch: ported forward for merge
 -- Dustin Kirkland <email address hidden> Tue, 01 Dec 2009 21:09:28 -0600

[Dustin Kirkland  (kirkland)]

Dan/Thierry-

I just uploaded a package to karmic-proposed containing the eucalyptus r946 fix.

Could one of you guys help draft the SRU testing notes and put that into the bug description? Thanks.

[Martin Pitt (pitti)]

Accepted eucalyptus into karmic-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[Dustin Kirkland  (kirkland)]

Thierry-

Can you verify this package?

We need other fixes associated with this update here onsite at a partner.

[Thierry Carrez (ttx)]

Dustin,
We can't verify this fix without the associated Rampart fix, which is pending some tests on Dan/Eucalyptus side.

[Daniel Nurmi (nurmi)]

All,

I've verified that the upstream patch, on top of the patch we've provided, is both functional and does not exhibit the memory leak problem; the test was to run the CC/NC against patched rampart/c for approximately 18 hours, watching the memory footprint of the associated apache2 processes.

[Thierry Carrez (ttx)]

Minimal patch for rampart

[Launchpad Janitor (janitor)]

This bug was fixed in the package rampart - 1.3.0-0ubuntu7

---------------
rampart (1.3.0-0ubuntu7) lucid; urgency=low

  * debian/patches/rampart-memleak.patch: Fix memory leak in rampart where
    for every connection receiver_cert is not freed (LP: #460085)
 -- Thierry Carrez <email address hidden> Mon, 14 Dec 2009 09:09:58 +0100

[Martin Pitt (pitti)]

Accepted rampart into karmic-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[Daniel Nurmi (nurmi)]

All,

I've been able to confirm that the newest euca/rampart packages, from proposed, are both functional and do not exhibit the memory leak. To be specific, i'm running with:

eucalyptus-cc: 1.6~bzr931-0ubuntu7.4
librampart0: 1.3.0-0ubuntu5.1

Thanks!
-Dan

[Launchpad Janitor (janitor)]

This bug was fixed in the package eucalyptus - 1.6~bzr931-0ubuntu7.4

---------------
eucalyptus (1.6~bzr931-0ubuntu7.4) karmic-proposed; urgency=low

  [ Thierry Carrez ]
  * cluster/handlers.c: Cherrypick upstream r946: initialize axis2c in the CC
    for each NC client connection, to avoid rampart memory leak (LP: #460085)
  * clc/modules/wsstack/src/main/java/com/eucalyptus/ws/handlers/HmacV2Handler.java:
    Cherrypick upstream r1079: Fix authentication issue when using a euca2ools
    that doesn't double base64encode userdata (LP: #461156)

  [ Dustin Kirkland ]
  * debian/eucalyptus-cc.upstart, debian/eucalyptus-common.eucalyptus.upstart:
    support CLEAN=1 on start/stop/restart of eucalyptus/euclayptus-cc; export
    the CLEAN env variable in eucalyptus.init, and handle it in both the
    pre-start and post-stop sections of eucalyptus-cc, (LP: #491254)
 -- Dustin Kirkland <email address hidden> Wed, 02 Dec 2009 17:58:18 -0600

[Launchpad Janitor (janitor)]

This bug was fixed in the package rampart - 1.3.0-0ubuntu5.1

---------------
rampart (1.3.0-0ubuntu5.1) karmic-proposed; urgency=low

  * debian/patches/rampart-memleak.patch: Fix memory leak in rampart where
    for every connection receiver_cert is not freed (LP: #460085)
 -- Thierry Carrez <email address hidden> Mon, 14 Dec 2009 08:43:06 +0100