GNOME Bugzilla – Bug 149845
GUI freezes when accessing gtk widgets from a thread
Last modified: 2005-08-22 12:49:38 UTC
When operating on certain widgets in a certain way inside a thread the GUI freezes. I.e. self.textbuffer.get_end_iter() will freeze the gui, but self.textbuffer.insert_at_cursor() will not. Uncomment the line in this code to reprocude the bug: import pygtk pygtk.require("2.0") import gtk import threading import time class MyThread(threading.Thread): def __init__(self, textbuffer): threading.Thread.__init__(self) self.stopping = 0 self.textbuffer = textbuffer def run(self): while not self.stopping: gtk.threads_enter() # The line below will freeze the app #iter = self.textbuffer.get_end_iter() gtk.threads_leave() gtk.threads_enter() self.textbuffer.insert_at_cursor("hi") gtk.threads_leave() time.sleep(1) def stop(self): self.stopping = 1 class Problem: def __init__(self): window = gtk.Window(gtk.WINDOW_TOPLEVEL) window.connect("destroy", self.destroy) button = gtk.Button("Start thread") textview = gtk.TextView() textbuffer = gtk.TextBuffer() textview.set_buffer(textbuffer) box = gtk.VBox() box.pack_start(button) box.pack_start(textview) window.add(box) self.thread = MyThread(textbuffer) button.connect("clicked", self.start_thread, None) window.show_all() def start_thread(self, widget, data=None): self.thread.start() def destroy(self, widget): self.thread.stop() gtk.main_quit() gtk.threads_init() test = Problem() gtk.threads_enter() gtk.main() gtk.threads_leave()
I've tried your code with pygtk 2.2.0, 2.3.94, 2.3.96 and CVS HEAD. The problem occurs with 2.3.96 and CVS HEAD, but not with 2.2.0 and 2.3.94. Anybody can confirm this?
I'm not sure if this is the correct fix, as i'm really a C newbie, and never hacked pygtk code, but I found out that applying this patch (http://www.gnome.org/~johan/pygboxed-threading.diff) provided by johan, and after that, applying this small one, will fix *this* problem. Please note that this small patch is mine, and in this case, it could break everything else. It would be good if any pygtk hacker give some comments on this. Here's the second patch: --- pygboxed-orig.c 2004-08-31 11:32:23.000000000 -0300 +++ pygboxed.c 2004-08-31 11:36:49.000000000 -0300 @@ -30,9 +30,9 @@ pyg_boxed_dealloc(PyGBoxed *self) { if (self->free_on_dealloc && self->boxed) { - PyGILState_STATE state = pyg_gil_state_ensure(); + pyg_begin_allow_threads; g_boxed_free(self->gtype, self->boxed); - pyg_gil_state_release(state); + pyg_end_allow_threads; } self->ob_type->tp_free((PyObject *)self);
This is the result of a bug in the python 2.3 core, which is fixed for 2.4 CVS; see http://python.org/sf/1010677 pygtk needs to work around this python 2.4 isn't out yet. The good news is that pygtk is already using its own names for the functions so the fix should only be to the implementation of the functions. I suspect the thing to do is basically copy the implementation of PyGILState_Ensure, but to store the PyThreadState pointer in our own thread local storage and when the pointer is NULL, to traverse thread state linked list found in the interpreter object. The hard part may be figuring out when a thread is destroyed.
Somehow I _knew_ PyGIL* API was buggy. I remember mentioning that in #pygtk. ;-) I think that copying PyGILState_Ensure implementation is just hairy and ugly. This is a grave bug in python. I'd prefer to just pressure python maintainers to backport this fix to python 2.3, then make pygtk require python 2.3.5 when it comes out.
Actually, this comment probably means the fix will be backported, so all we have to do is wait. Date: 2004-08-24 14:14 Sender: anthonybaxter Logged In: YES user_id=29957 Boosting to pri 7 for pre-2.4 inclusion.
Created attachment 31194 [details] [review] Preliminary fix Here's a preliminary patch which fixes the problem with threads created via the threading module, but doesn't create new thread state objects for threads which are unknown to the Python runtime. I also need to think through if there are problems with code outside of pygtk that does use PyGILState_Ensure, though even if there is, that code is likely to have problems anyway. Still to do: * Creating new thread state objects when needed * Adding debugging checks * Determining if there is a better way to iterate through the known thread states; I'm not convinced the current code is thread safe. This assumes there's only one interpreter state, but I've never seen multiple interpreter states used and I think there have been comments on python-dev that the current multi interpreter state support doesn't really work.
I'm starting to doubt that the functions to enumerate threads are thread safe because nothing seems to prevent the destruction of the state objects returned by another thread. I'll try to get an answer from the python core developers on this. How was the thread state determined before pygtk switched to the GILState functions?
> This assumes there's only one interpreter state, [...] Funny that the other day owen was pointing out that pygtk doens't work well embedded in xchat, because the xchat python interface keeps one interpreter state per plugin. Specifically, idle and timeout functions seem to cause a deadlock. I personally hate the python xchat plugin, though...
Created attachment 31582 [details] [review] New fix for 2.3 Updated fix that assumes the GIL is locked if the thread state for a thread can't be determined, which is what prior pygtk releases did (I think). The state for any thread that calls pyg_thread_enable is saved so it can be properly restored -- I modified pyg_thread_enable so that it may be called multiple times. I added a conditional call to gtk_main so that thread this is invoked on gets recorded The example in the bug comment works on Linux, but I haven't tested beyond that.
*** Bug 149741 has been marked as a duplicate of this bug. ***
This patch looks like it is probably the best bet for Python versions < 2.3.5. However, the check used when setting PYGIL_API_IS_BUGGY looks like it might be wrong: +#if PY_HEXVERSION < 0x0204a400 +#define PYGIL_API_IS_BUGGY +#endif Isn't the hexversion for 2.4a4 0x020400a4? One thing that might be worth doing is to check the Python version at runtime if Python 2.3 is being used, and use the PyGILState APIs if it is new enough. However, since 2.3.5 isn't available yet, this shouldn't hold up checking the fix in. To check the running Python version, the following code should work: py_hexver = PySys_GetObject("hexversion"); hexver = PyInt_AsLong(py_hexver); gilstate_okay = hexver >= 0x020305a0
Yes, you're right that the HEXVERSION check should be 0x020400a4 -- although we could just use 0x02040000 and ignore the bugs in the alphas. The next question is whether we want to switch implementations for 2.3.5. My take is that I would want all 2.3.x versions to act the same, but then I don't actually use pygtk in a multithreaded app.
Well, the reason for using PyGILState in the first place is to get interoperability with other modules that have similar callback/reentrancy issues to pygtk. One example is PyORBit. While it would be possible to get it to cooperate by depending on pygobject and using its threading enter/leave calls it would be nicer if they didn't need to know about each other. The idea of PyGILState was to push the primitives needed for this kind of cooperation down to the Python API itself. So in theory it would be nice to use PyGILState where it works, but if it isn't realistic to interoperate with other PyGILState-using modules in 2.3, then it isn't as big a deal.
Using PyGILState is a good thing. The question is whether programs should act differently (with the inevitable different set of bugs) on 2.3.5 versus 2.3.4. I'd say no, but it is a debatable question.
That is a good point. However, at the same time we have the behaviour of Python changing between 2.3.4 and 2.3.5, meaning the branch maintainer thought the fix was important or small enough to go into a micro release (I'm not sure which). To tell the truth, I'm not sure how many packages would benefit from PyGTK using PyGILState APIs when running with Python 2.3.5. No other stable pygtk release cooperates properly with other modules that have callbacks/reenter, so it isn't a regression ...
I think it was backported because 2.3.4 can easily lock up when PyGILState api's were used, so there's no risk of making things worse. This also tells me that not many people use PyGILState in 2.3. How about not supporting PyGILState in 2.3.x for now? If 2.3.5 comes out before 2.4.0 and there is demand to support PyGILState in 2.3.5, pygtk can then support it. There's no timeline for releasing 2.3.5 that I'm aware of. The point of this patch is to remove a regression in pygtk when running with Python 2.3.x
This was committed in CVS head, however I found a regression which is committed as a testcase, make check should trigger it. (use gdb runtest.py to see the backtrace): Thread 1:
+ Trace 50541
Note, if I remove gobject.threads_init() in test_thread.py it works fine again.
Created attachment 32011 [details] [review] Enable it in runtime This fixes the regression I found yesterday by adding a usegil keyword argument to gobject.threads_init(). Default is to use the old 2.2 like API and only use GIL if usegil is passed or a 2.4.x or 2.3.5 is used.
Unfortunately, the test program at the top of this bug report deadlocks, at least on python 2.3.4. (why was this bug not confirmed?..)
Created attachment 32016 [details] [review] Take two runtime v2: Cleanup suggestions from james and make it actually work for old programs.
This is fixed.