Bug 237179

Summary: flags() method on GtkWidget leaks memory
Product: [Fedora] Fedora Reporter: Daniel BerrangĂ© <berrange>
Component: pygobject2Assignee: Matthew Barnes <mbarnes>
Status: CLOSED CURRENTRELEASE QA Contact:
Severity: medium Docs Contact:
Priority: medium    
Version: 6CC: mpg
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: pygobject2-2.12.3-2.fc6 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-04-20 01:45:01 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Fix memory leak none

Description Daniel Berrangé 2007-04-19 20:58:46 UTC
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 Berrangé 2007-04-19 20:58:46 UTC
Created attachment 153065 [details]
Fix memory leak

Comment 2 Matthew Barnes 2007-04-20 01:19:15 UTC
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-20 01:20:03 UTC
[1] http://docs.python.org/api/dictObjects.html

Comment 4 Matthew Barnes 2007-04-20 01:45:01 UTC
Fixed in pygobject2-2.12.3-2.fc6 and pygobject2-2.12.3-3.fc7.