Bug 644824 - Backport fixes for memory leaks from OpenMotif 2.3.3
Backport fixes for memory leaks from OpenMotif 2.3.3
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: openmotif (Show other bugs)
5.5
All Linux
medium Severity high
: rc
: ---
Assigned To: Thomas Woerner
Lukas Zachar
: Patch
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-10-20 07:51 EDT by Olivier Fourdan
Modified: 2013-04-12 16:46 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-11-14 04:29:34 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Proposed patch (1.55 KB, patch)
2010-10-20 07:51 EDT, Olivier Fourdan
no flags Details | Diff
Patch to plug more leaks (32.70 KB, patch)
2010-11-26 11:57 EST, Olivier Fourdan
no flags Details | Diff
Simpler patch to plug Trait leaks (976 bytes, patch)
2010-11-29 08:38 EST, Olivier Fourdan
no flags Details | Diff


External Trackers
Tracker ID Priority Status Summary Last Updated
MotifZone 1388 None None None Never

  None (edit)
Description Olivier Fourdan 2010-10-20 07:51:02 EDT
Created attachment 454558 [details]
Proposed patch

Description of problem:

Customer is facing a memory leak that have been addressed in Openmotif 2.3.3 upstream and requests these fixes to be backported in Openmotif 2.3.1 on Red Hat Emterprise Linu 5.

Version-Release number of selected component (if applicable):

openmotif-2.3.1-2.el5_4.1

How reproducible:

Always

Steps to Reproduce:

1. Get the source example from http://bugs.motifzone.net/show_bug.cgi?id=1411
2. Build the binary
3. Run in valgrind

Actual results:

There is a leak induced by the ToolTip.

==15284== 128 bytes in 8 blocks are definitely lost in loss record 516 of 666
==15284==    at 0x4A05140: calloc (vg_replace_malloc.c:418)
==15284==    by 0x321FC133EC: XtCalloc (Alloc.c:161)
==15284==    by 0x323FBE2A87: XmSetToolTipString (ToolTip.c:333)
==15284==    by 0x323FB678A2: Initialize (Primitive.c:782)
==15284==    by 0x321FC1B368: CallInitialize (Create.c:219)
==15284==    by 0x321FC1B337: CallInitialize (Create.c:214)
==15284==    by 0x321FC1BDE5: xtCreate (Create.c:409)
==15284==    by 0x321FC1C77B: _XtCreateWidget (Create.c:563)
==15284==    by 0x321FC4B248: _XtVaCreateWidget (VarCreate.c:84)
==15284==    by 0x321FC4B473: XtVaCreateManagedWidget (VarCreate.c:139)
==15284==    by 0x400ADA: handler (in /root/repro)
==15284==    by 0x321FC2DC76: DoOtherSources (NextEvent.c:1142)
==15284== 
==15284== 128 bytes in 8 blocks are definitely lost in loss record 517 of 666
==15284==    at 0x4A05140: calloc (vg_replace_malloc.c:418)
==15284==    by 0x321FC133EC: XtCalloc (Alloc.c:161)
==15284==    by 0x323FBE2A87: XmSetToolTipString (ToolTip.c:333)
==15284==    by 0x323FB678A2: Initialize (Primitive.c:782)
==15284==    by 0x321FC1B368: CallInitialize (Create.c:219)
==15284==    by 0x321FC1B337: CallInitialize (Create.c:214)
==15284==    by 0x321FC1BDE5: xtCreate (Create.c:409)
==15284==    by 0x321FC1C77B: _XtCreateWidget (Create.c:563)
==15284==    by 0x321FC1CB4D: XtCreateWidget (Create.c:582)
==15284==    by 0x323FBE2B21: ToolTipGetData (ToolTip.c:61)
==15284==    by 0x323FBE2C65: _XmToolTipLeave (ToolTip.c:266)
==15284==    by 0x323FB677CD: SetValues (Primitive.c:954)


Expected results:

No leak

Additional info:

The leak was fixed by the second part of the fix for bug 1388:

    http://bugs.motifzone.net/showattachment.cgi?attach_id=251

Proposed patch attached (this patch comes from the diff between ToolTip.c and VendorS.c between Openmotif 2.3.1 and 2.3.3)
Comment 1 Olivier Fourdan 2010-11-26 11:57:14 EST
Created attachment 463103 [details]
Patch to plug more leaks

Even with he upstream patches included, our customer still reports the following leaks:

==1527== 8 bytes in 1 blocks are possibly lost in loss record 5,526 of
 59,799

==1527==    at 0x8020C5E: malloc (vg_replace_malloc.c:236)

==1527==    by 0xE906E07: XtMalloc (Alloc.c:127)

==1527==    by 0xE7D5AF8: XmeTraitSet (Trait.c:204)

==1527==    by 0xE7C97D6: ClassPartInitialize (VendorS.c:830)

==1527==    by 0xE90F2A6: CallClassPartInit (Create.c:85)

==1527==    by 0xE90F29A: CallClassPartInit (Create.c:82)

==1527==    by 0xE90F6F2: XtInitializeWidgetClass (Create.c:193)

==1527==    by 0xE90F7C5: XtInitializeWidgetClass (Create.c:189)

==1527==    by 0xE90FA47: xtWidgetAlloc (Create.c:268)

==1527==    by 0xE90FC2C: xtCreate (Create.c:357)

==1527==    by 0xE9101A4: _XtAppCreateShell (Create.c:698)

==1527==    by 0xE9103DB: XtAppCreateShell (Create.c:725)

==1527==    by 0xE91B759: XtOpenApplication (Initialize.c:990)

==1527==    by 0xE91B800: XtAppInitialize (Initialize.c:1015)

==1527==    by 0x892480C: initialise_toolkit (initialise.c:143)

==1527==    by 0x89439AE: motif__initialise (motif.adb:522)

...

==1527== 8 bytes in 1 blocks are possibly lost in loss record 5,527 of
 59,799

==1527==    at 0x8020C5E: malloc (vg_replace_malloc.c:236)

==1527==    by 0xE906E07: XtMalloc (Alloc.c:127)

==1527==    by 0xE7D5AF8: XmeTraitSet (Trait.c:204)

==1527==    by 0xE7C97F7: ClassPartInitialize (VendorS.c:833)

==1527==    by 0xE90F2A6: CallClassPartInit (Create.c:85)

==1527==    by 0xE90F29A: CallClassPartInit (Create.c:82)

==1527==    by 0xE90F6F2: XtInitializeWidgetClass (Create.c:193)

==1527==    by 0xE90F7C5: XtInitializeWidgetClass (Create.c:189)

==1527==    by 0xE90FA47: xtWidgetAlloc (Create.c:268)

==1527==    by 0xE90FC2C: xtCreate (Create.c:357)

==1527==    by 0xE9101A4: _XtAppCreateShell (Create.c:698)

==1527==    by 0xE9103DB: XtAppCreateShell (Create.c:725)

==1527==    by 0xE91B759: XtOpenApplication (Initialize.c:990)

==1527==    by 0xE91B800: XtAppInitialize (Initialize.c:1015)

==1527==    by 0x892480C: initialise_toolkit (initialise.c:143)

...

==1527== 8 bytes in 1 blocks are possibly lost in loss record 5,528 of
 59,799

==1527==    at 0x8020C5E: malloc (vg_replace_malloc.c:236)

==1527==    by 0xE906E07: XtMalloc (Alloc.c:127)

==1527==    by 0xE7D5AF8: XmeTraitSet (Trait.c:204)

==1527==    by 0xE7C9818: ClassPartInitialize (VendorS.c:836)

==1527==    by 0xE90F2A6: CallClassPartInit (Create.c:85)

==1527==    by 0xE90F29A: CallClassPartInit (Create.c:82)

==1527==    by 0xE90F6F2: XtInitializeWidgetClass (Create.c:193)

==1527==    by 0xE90F7C5: XtInitializeWidgetClass (Create.c:189)

==1527==    by 0xE90FA47: xtWidgetAlloc (Create.c:268)

==1527==    by 0xE90FC2C: xtCreate (Create.c:357)

==1527==    by 0xE9101A4: _XtAppCreateShell (Create.c:698)

==1527==    by 0xE9103DB: XtAppCreateShell (Create.c:725)

==1527==    by 0xE91B759: XtOpenApplication (Initialize.c:990)

==1527==    by 0xE91B800: XtAppInitialize (Initialize.c:1015)

==1527==    by 0x892480C: initialise_toolkit (initialise.c:143)

...


Repeated several times. Plus a few others, all pointing to XmeTraitSet().

It seems that if ClassPartInitialize() is called several times, the Trait is added each time.

Actually, Traits seems to be added to the Hash table in any case without prior checking if the Trait is already present, but are never removed, and that's the case in many parts of the code.

The following patch tries to avoid this. But please beware, this patch has received very light testing, I just checked that a few Motif programs still work and do not crash and burn. 

Given that the same or similar code is to be found in many different sources, the resulting patch is fairly big and touches a lot of sources.

I shall provide a test package to our customer so he can try with his application as well and verify if the leaks are not reported anymore with that.

PS: This patch goes on top of the previous one (it does not replace it).
Comment 2 Olivier Fourdan 2010-11-29 08:38:59 EST
Created attachment 463485 [details]
Simpler patch to plug Trait leaks

Actually, I think there is a much simpler way to address these Trait leaks.

The traits are stored in a hash table with the key made of the object and the property.

Therefore having more than one entry in that hash table with the same key would not make sense (and would indeed introduce a leak), which means that one object can have only one entry for a given property.

Instead of changing the code in each portion that adds traits, this new patch changes only Xm/Trait.c to check for an existing key before adding the same one again.
Comment 3 Olivier Fourdan 2010-11-29 09:51:39 EST
Comment on attachment 463485 [details]
Simpler patch to plug Trait leaks

Marking patch as obsolete, it does not address the leak (which is, Traits are added but never removed, even when the objects are destroyed)
Comment 4 Olivier Fourdan 2010-11-30 04:28:13 EST
I am not entirely convinced the problem reported in comment #1 is a true leak (unlike the initial issue reported in comment #0). A leak would be if new memory was allocated while references to previous allocated memory was lost, ie the memory usage would grow over time.

Here what we see is an allocation that occurs once, it's not freed, but it's not reallocated either.

This is because the traits reported to leak here are set to the widget class, ie the first time a widget of a given class is created, the widget class gets initialized and ClassPartInitialize() is called. ClassPartInitialize() will add traits for that particular widget class and store them in a hash table using a key made of the object (the widget class) and the name of the property.

If the widget is destroyed, the entry is not freed nor removed from the hash table. Yet when a new widget from the same class is created, the same traits are not added again, the previous allocated traits for that particular widget class/property remain and are reused. 

For example, creating a widget and destroying it will add a few Traits, but creating a 1000 of the same type of widgets will not create a 1000 more traits. 

Beside, I see no hook in the API for destroying the widget class so I see no way to remove the traits added at initialization. Of course we see in valgrind that the same sequence in VendorS.c is called more than once, but checking with GDB, we can see that it's for different Widget Classes, there is just one sequence for a given widget class.

To confirm this, run the reproducer for a 1000 iterations and check the leak summary:

==27404== LEAK SUMMARY:
==27404==    definitely lost: 60 bytes in 1 blocks
==27404==    indirectly lost: 0 bytes in 0 blocks
==27404==      possibly lost: 34,844 bytes in 79 blocks
==27404==    still reachable: 549,337 bytes in 3,021 blocks
==27404==         suppressed: 0 bytes in 0 blocks

Redo the same and leave it running for just a just 5 iterations and compare the results:

==27657== LEAK SUMMARY:
==27657==    definitely lost: 60 bytes in 1 blocks
==27657==    indirectly lost: 0 bytes in 0 blocks
==27657==      possibly lost: 34,844 bytes in 79 blocks
==27657==    still reachable: 514,273 bytes in 2,021 blocks
==27657==         suppressed: 0 bytes in 0 blocks

The values for definitely/indirectly/possibly lost are identical, there is no additional leak reported by repeating the creation/destruction of widgets, apparently.

Note, this is only for the problem reported in comment #1, the leak reported initially in comment #0 is real and being addressed by the proposed patch (attachment 454558 [details]) which is the fix from upstream.
Comment 8 RHEL Product and Program Management 2011-05-31 09:34:13 EDT
This request was evaluated by Red Hat Product Management for
inclusion in the current release of Red Hat Enterprise Linux.
Because the affected component is not scheduled to be updated in the
current release, Red Hat is unfortunately unable to address this
request at this time. Red Hat invites you to ask your support
representative to propose this request, if appropriate and relevant,
in the next release of Red Hat Enterprise Linux.
Comment 12 errata-xmlrpc 2011-11-14 04:29:34 EST
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2011-1451.html

Note You need to log in before you can comment on or make changes to this bug.