[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [patch] python locking fixes
A Sex, 2003-10-03 às 12:55, Marko Kreen escreveu:
> On Thu, Oct 02, 2003 at 11:48:50PM +0100, Gustavo J. A. M. Carneiro wrote:
> > A Qui, 2003-10-02 às 22:08, Marko Kreen escreveu:
> > > Making python lock recursive was not a good idea because
> > > it assumed that next callback will be from same plugin.
> > > And anyway, the recursive locks were only needed because
> > > xchat and python locks were both held at the same time.
> >
> > Are you sure about this? Consider this example plugin:
> >
> > import gtk
> >
> > def someXchatHook(...):
> > dialog = gtk.Dialog(...)
> > dialog.run()
> >
> > It may run into a deadlock, like this:
> >
> > 1. xchat hook triggers plugin
> > 2. acquire python interpreter lock
> > 3. create gtk dialog
> > 4. dialog.run(): runs a recursive gtk main loop; but notice that gtk
> > main loop and xchat main loop are one and the same; meanwhile:
> > 4a. some xchat event triggers another hook in this plugin
> > 4b. acquire python interpreter lock, but
> > -> lock already acquired (by the same thread) => deadlock!
>
> I checked pygtk 2.0.0 sources: it seems to have similar ideas
> about locking as patched python.c - dialog.run() drops python
> lock before calling gtk_dialog_run. So no deadlock.
If you look at _wrap_gdk_threads_init() in gdk.override, you'll notice
that python locking acquire and release only happens if all the
following are true:
1. pygtk was built with threading (--enable-thread), which is enabled
by default if the python thread module is found;
2. The python code calls gtk.threads_init().
If 1 and 2 are not true, and if I'm reading the code correctly, then
pyg_unblock_threads and pyg_block_threads do effectively nothing.
I have just realized this now, and that leads me to the conclusion
that *probably* the deadlock problems disappear if I call
gtk.threads_init(). I also found this FAQ:
http://www.async.com.br/faq/pygtk/index.py?req=show&file=faq20.006.htp
> But lets assume python.c uses recursive locking. So,
> entering step 4b without Python lock, python.c thinks it still
> has it and wont bother grabbing it. So it starts doing Python
> stuff without valid thread state -> crash. The recursion
> lets xchat get out of sync with reality.
>
>
> I did a quick research on pygtk locking and it _seems_ the
> important signal multiplexing funtions like main, mainloop,
> dialog_run use locking similar to my patch so everything should
> work. But there are also some hints of danger - they use
> their own variant of 'recursive Python lock' - lots of smaller
> funtions like 'set_text' dont do Python locking but may emit
> signals so interpreter can be entered recursively.
> That still wont give us problems, sequence
>
> xchat->python(1)->gtk(*)->python(2)
>
> where (*) wont drop python lock wont give us problems,
> python(2) can call both gtk and xchat as much it wants.
> Problematic sequence is:
>
> xchat->python(1)->gtk(*)->xchat [X]
>
> ok, it seems possible to solve it with recursive lock. But now:
>
> xchat->python->gtk_dialog_run(drops lock)->xchat [1]
>
> so with or without recursive lock we are pooped, if [X] can
> happen.
>
> pygtk source has files THREADING and TODO which talk
> about threading but they seem to be out of date, but looking
> at them and the source I would say that pygtk authors intent
> is also not to let [X] happen and it should be considered
> a bug in pygtk.
>
> Only way to let [X] happen safely is when both xchat and
> pygtk use _same_ recursive python lock (currently each has its
> own housekeeping variables). It is possible in the future:
> starting from Python 2.3 there are such funstions in Python,
> PyGILState_Ensure, PyGILState_Release. And pytgtk 2.2 _may_
> start useing them, so then xchat can be converted too (remember,
> _both_ need to be using them).
This is very interesting. The relevant PEP is here:
http://www.python.org/peps/pep-0311.html
James Henstridge has already manifested intention of using python
2.3's GIL functions in the development branch of pygtk:
http://www.daa.com.au/pipermail/pygtk/2003-September/005827.html
Quoting James Henstridge:
"There is one big improvement in Python 2.3 that would be worth making
use of: the new PyGILState APIs, which could significantly reduce the
complexity of PyGTK's thread support (and get it to play nicer with
other threading aware Python extensions):
http://www.python.org/peps/pep-0311.html
"
>
> Hmm. I wanted to continue with "that messes xchat_lock
> handling up", but now I wondering if its correct even now,
> in light of [1]. Need to think a bit. But I still dont
> see anything wrong in my handling of Python lock.
>
> > > Following patch fixes locking, so that only one of the
> > > locks is held at any time - except in one controlled
> > > situation. That means when calling xchat code, python
> > > lock is dropped and when calling python code, xchat lock
> > > is dropped. Little bit of code reorg was needed, to better
> > > separate xchat and python code.
> >
> > I see what you're trying to do. You are thinking that if you release
> > the python lock before calling xchat functions, then if the plugin
> > reenters due to the xchat call, the lock would be free to be acquired
> > again. But you are forgetting that reentry can be caused by pygtk
> > functions too. I don't think there's any other way to make this work
> > without a recursive mutex.
>
> But xchat locking is now "correct" in the sense that whether
> this works or not depends on pygtk locking. And if pygtk
> does not play fair, then adding recursion to locking does not
> help because recursion variables will show nonsense.
Ok, in light of these new discoveries, I see nothing wrong with your
patch. However, I don't think things will ever work smoothly until both
pygtk and xchat-python migrate to the Python GIL API. The great thing
about the GIL API is that the underlying mutex is recursive. I don't
mind waiting though. In the mean time I can use my forked python plugin
for my own plugins, and later move to xchat's own python plugin when it
works for me.
Anyway, thanks for looking into this! :-)
Regards,
--
Gustavo João Alves Marques Carneiro
<gjc@inescporto.pt> <gustavo@users.sourceforge.net>
--
XChat-discuss: mailing list for XChat users
Archive: http://mail.nl.linux.org/xchat-discuss/