gtk_main leaks a file descriptor every time the main loop is run if threads have been initialized

[Glyph Lefkowitz]

Here is a Python script, using pygtk, which demonstrates the problem.

from gtk import main, main_quit
from gobject import threads_init, timeout_add, source_remove
import sys

def test(iterations, use_threads):
    if use_threads:
        threads_init()
    tick = 0
    while tick < iterations:
        tick += 1
        if (tick % 1000) == 0:
            print 'Tick', tick
        lasttag = timeout_add(1, main_quit)
        main()
        source_remove(lasttag)

if __name__ == "__main__":
    if sys.argv[1:] == ['no-threads']:
        threads = False
    else:
        threads = True
    print 'Using threads:', threads
    test(10000, threads)
    print 'Done.'

When run on Jaunty or Intrepid, this is the output:

$ python gtkrash.py
Using threads: True

** ERROR **: Cannot create main loop pipe: Too many open files

aborting...
Aborted
$ python gtkrash.py no-threads
Using threads: False
Tick 1000
Tick 2000
Tick 3000
Tick 4000
Tick 5000
Tick 6000
Tick 7000
Tick 8000
Tick 9000
Tick 10000
Done.
$

When run on Hardy, however, the test is successful regardless of whether threads are used.

[Sebastien Bacher (seb128)]

thank you for your bug report, is that specific to pygtk or do you have it also in C GTK? the issue is an upstream one and should be open on bugzilla.gnome.org when the people writting pygtk or gtk will read it

[Glyph Lefkowitz (glyph)]

Usually Python programs are pretty good approximations of their C counterparts, but in this case the bug is in PyGTK. It's on line 1098 of gtk.override from the pygtk package.

Actually the bug has been in PyGTK for a long time. It was in the version distributed in hardy. The reason it's cropping up in Jaunty is that Python 2.6 has the "PySignal_SetWakeupFd" function that has to be defined in order for the buggy code path to get invoked. Intrepid uses 2.5, but has this function backported in a patch.

The correct solution would be to inspect the return value of PySignal_SetWakeupFd and restore it (and close the signal pipe) when the gtk main loop exits, rather than throwing it away and calling pipe() every time.

[Glyph Lefkowitz (glyph)]

Filed the bug upstream as http://bugzilla.gnome.org/show_bug.cgi?id=579406

[Sebastien Bacher (seb128)]

the bug has been fixed upstream now

[Sebastien Bacher (seb128)]

the bug is fixed in the karmic version now

[Nicola (nicola)]

any chance to see the fix in jaunty too?

[Nicola (nicola)]

I can reproduce the same error using gobject only without gtk import so I think the upstream fix don't really solve the problem:

# -*- coding: utf-8 -*-
from gobject import threads_init, timeout_add, source_remove
import gobject
import sys

def test(iterations, use_threads):
    if use_threads:
        threads_init()
    tick = 0
    while tick < iterations:
        loop=gobject.MainLoop()
        tick += 1
        if (tick % 1000) == 0:
            print 'Tick', tick
        lasttag = timeout_add(1, loop.quit)
        loop.run()
        source_remove(lasttag)

if __name__ == "__main__":
    if sys.argv[1:] == ['no-threads']:
        threads = False
    else:
        threads = True
    print 'Using threads:', threads
    test(10000, threads)
    print 'Done.'

Using threads: True

** ERROR **: Cannot create main loop pipe: Too many open files

aborting...
Aborted

[Glyph Lefkowitz (glyph)]

Nicola, you seem to be right. From what I can tell, the offending code is very nearly copy/pasted into pygobject in pygmainloop.c:pyg_signal_watch_prepare. As far as I can see, pygtk should be invoking this code, not duplicating it.

[Sebastien Bacher (seb128)]

The bug should be sent upstream by somebody having the issue

[Nicola (nicola)]

reported:

https://bugzilla.gnome.org/show_bug.cgi?id=579406#c2

Nicola

[Jean-Paul Calderone (exarkun)]

Upstream has fixed this second issue as well. Here's the change: http://git.gnome.org/browse/pygobject/commit/?id=d042402b7c649b2bed7f20038eb82518ec7cc9b3

It would be excellent to see this second half of the fix backported.

[Sebastien Bacher (seb128)]

do you have a testcase showing the issue on lucid?

[Jean-Paul Calderone (exarkun)]

Yes, but not a minimal one. So far I've been unable to reproduce the problem with a smaller test than this:

    trial -u twisted.internet.test.test_tcp.TCPClientTestsBuilder_Glib2Reactor.test_addresses

The expected behavior is for this to execute 1021 runs of the specified unit test (from Twisted, so this requires python-twisted to be installed, but it may be by default in Lucid). On the 1021st run, it will hang for 30 seconds and then exit with an error.

I'm aware that a more self-contained test case would be more desirable, but this at least demonstrates that a problem exists.

The behavior is better on Lucid than in previous releases, where the leaks is about twice as fast and the run fails after only 507 iterations. I'd love to see enough of the related fixes backported to karmic to fix this issue as well.

[Sebastien Bacher (seb128)]

the change on comment #11 is already in the lucid version, is that still an issue there?

[Jean-Paul Calderone (exarkun)]

Two things seem to be getting confused here.

1) The Lucid version performs better than any of the previously packaged versions. So, I'd like the fixes which are present in the Lucid package to be backported to Karmic and Hardy.

2) Separately, the Lucid version still leaks file descriptors, probably because of some reference cycle which gobject can't deal with.

Sorry for not being more clear earlier.

Can the fixes which are in the Lucid package be backported to Karmic and Hardy?

[Jean-Paul Calderone (exarkun)]

Huh? Can you please add a comment with that "Invalid" designation.

[Glyph Lefkowitz (glyph)]

Since this bug has come to my attention again, I should note that the behavior that Jean-Paul described in comment 15 is still behaving that way on oneiric. "trial -u twisted.internet.test.test_tcp.TCPClientTestsBuilder_Glib2Reactor.test_addresses" still bombs out on the 1021st run. I guess we need to report a separate bug for that, since the original reproducer no longer appears to crash?