Bug 237179 - flags() method on GtkWidget leaks memory
flags() method on GtkWidget leaks memory
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: pygobject2 (Show other bugs)
6
All Linux
medium Severity medium
: ---
: ---
Assigned To: Matthew Barnes
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-04-19 16:58 EDT by Daniel Berrange
Modified: 2007-11-30 17:12 EST (History)
1 user (show)

See Also:
Fixed In Version: pygobject2-2.12.3-2.fc6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-04-19 21:45:01 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Fix memory leak (498 bytes, patch)
2007-04-19 16:58 EDT, Daniel Berrange
no flags Details | Diff


External Trackers
Tracker ID Priority Status Summary Last Updated
GNOME Desktop 428726 None None None Never

  None (edit)
Description Daniel Berrange 2007-04-19 16:58:46 EDT
Description of problem:
If you call the flags() method on a GtkWidget it will leak memory on every
invocation. This method is called very frequently in virt-manager application
under certain conditions, causing our memory usage to balloon.

The following short demo program illustrates leaking of 10's of thousands of
objects every second:

import gobject
import gtk
import gc

win = gtk.Window()
win.show_all()

def foo():
    for i in range(1000):
        f = win.flags()
    gc.collect()

    types = {}
    for o in gc.get_objects():
        t = type(o)
        if types.has_key(t):
            types[t] += 1
        else:
            types[t] = 1
    for t in types.keys():
        if types[t] > 10000:
            print str(t) + " " + str(types[t])

    return True


gobject.timeout_add(100, foo)

gtk.main()


It will print out the type name of any object with more than 10,000 instances
every time foo() is called. The resu

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


How reproducible:
ALWAYS

Steps to Reproduce:
1. Run demo code above
2.
3.
  
Actual results:
Leaked objects:

# python demo.py 
<class 'gtk._gtk.WidgetFlags'> 10019
<class 'gtk._gtk.WidgetFlags'> 11019
<class 'gtk._gtk.WidgetFlags'> 12019
<class 'gtk._gtk.WidgetFlags'> 13019
<class 'gtk._gtk.WidgetFlags'> 14019
<class 'gtk._gtk.WidgetFlags'> 15019


Expected results:
No leaked objects

Additional info:
I traced the problem to the method:

pyg_flags_from_gtype (GType gtype, int value)


In particular this fragment of code:

    pyint = PyInt_FromLong(value);
    retval = PyDict_GetItem(values, pyint);
    Py_DECREF(pyint);

    if (!retval) {
	PyErr_Clear();

	retval = ((PyTypeObject *)pyclass)->tp_alloc((PyTypeObject *)pyclass, 0);
	g_assert(retval != NULL);
	
	((PyIntObject*)retval)->ob_ival = value;
	((PyGFlags*)retval)->gtype = gtype;
    }
    Py_INCREF(retval);


Notice that the Py_INCREF(retval) is called, regardless of whether the object
was obtained form the hash table, or newly allocated. In the former case it is
neccessary to increment the ref count, because the existing reference is related
to its use in the hashtable. In the latter case though, tp_alloc ensures the
refererence count is already suitably set to 1, and no other object has a
reference, so calling Py_INCREF(retval) causes the reference count to be too
large & thus leaks.

So I believe

    }
    Py_INCREF(retval);

Needs to change to

    } else {
      Py_INCREF(retval);
    }
Comment 1 Daniel Berrange 2007-04-19 16:58:46 EDT
Created attachment 153065 [details]
Fix memory leak
Comment 2 Matthew Barnes 2007-04-19 21:19:15 EDT
Yeah, looks like you're right.  According to the Python C API documentation [1],
PyDict_GetItem returns a borrowed reference, whereas tp_alloc() returns a new
reference (obviously).
Comment 3 Matthew Barnes 2007-04-19 21:20:03 EDT
[1] http://docs.python.org/api/dictObjects.html
Comment 4 Matthew Barnes 2007-04-19 21:45:01 EDT
Fixed in pygobject2-2.12.3-2.fc6 and pygobject2-2.12.3-3.fc7.

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