GNOME Bugzilla – Bug 620062
Python thread support is broken
Last modified: 2010-06-28 13:44:15 UTC
We should fix this, as it was done in totem and gedit.
http://git.gnome.org/browse/gedit/commit/?id=327384762b548bd80588cef9de2baa32a5c30a0a gedit fix.
Wanna port it over to libpeas ? ;-)
Created attachment 163609 [details] [review] [Python] Fix thread support.
Running Totem with one Python plugin (pythonconsole, which is non-threaded) enabled with attachment #163609 [details] applies to libpeas master gave the following crash: ** (totem:18955): WARNING **: Error opening directory '/home/philip/.local/share/totem/plugins': No such file or directory ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/skipto/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/pythonconsole/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/thumbnail/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/brasero-disc-recorder/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/opensubtitles/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/screenshot/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/youtube/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/sample-python/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/ontop/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/media-player-keys/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/jamendo/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/iplayer/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/sample-vala/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/coherence_upnp/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/gromit/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/dbus/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/screensaver/*.totem-plugin... ** (totem:18955): DEBUG: Loading /opt/gnome2/build/lib64/totem/plugins/properties/*.totem-plugin... Totem-Message: totem_plugins_engine_load_all Totem-Message: checking peas_plugin_info_get_module_name (info) movie-properties Totem-Message: peas_plugin_info_get_module_name (info) movie-properties, to activate Totem-Message: checking peas_plugin_info_get_module_name (info) screensaver Totem-Message: peas_plugin_info_get_module_name (info) screensaver, to activate Totem-Message: checking peas_plugin_info_get_module_name (info) dbus-service Totem-Message: checking peas_plugin_info_get_module_name (info) gromit Totem-Message: checking peas_plugin_info_get_module_name (info) coherence_upnp Totem-Message: checking peas_plugin_info_get_module_name (info) sample-vala Totem-Message: checking peas_plugin_info_get_module_name (info) iplayer Totem-Message: checking peas_plugin_info_get_module_name (info) jamendo Totem-Message: checking peas_plugin_info_get_module_name (info) media_player_keys Totem-Message: peas_plugin_info_get_module_name (info) media_player_keys, to activate Totem-Message: checking peas_plugin_info_get_module_name (info) ontop Totem-Message: checking peas_plugin_info_get_module_name (info) sample-python Totem-Message: checking peas_plugin_info_get_module_name (info) youtube Totem-Message: peas_plugin_info_get_module_name (info) youtube, to activate Totem-Message: checking peas_plugin_info_get_module_name (info) screenshot Totem-Message: peas_plugin_info_get_module_name (info) screenshot, to activate Totem-Message: checking peas_plugin_info_get_module_name (info) opensubtitles Totem-Message: checking peas_plugin_info_get_module_name (info) brasero-disc-recorder Totem-Message: checking peas_plugin_info_get_module_name (info) thumbnail Totem-Message: checking peas_plugin_info_get_module_name (info) pythonconsole Totem-Message: peas_plugin_info_get_module_name (info) pythonconsole, to activate Totem-Message: checking peas_plugin_info_get_module_name (info) skipto Totem-Message: peas_plugin_info_get_module_name (info) skipto, to activate ** (totem:18955): DEBUG: Loading loader 'c': '/opt/gnome2/build/lib64/libpeas-1.0/loaders/libcloader.so' ** (totem:18955): DEBUG: Registered extension for type 'PeasPluginLoader' ** (totem:18955): DEBUG: Insert module movie-properties into C module set ** (totem:18955): DEBUG: Registered extension for type 'PeasActivatable' ** (totem:18955): DEBUG: Insert module screensaver into C module set ** (totem:18955): DEBUG: Registered extension for type 'PeasActivatable' [New Thread 0x7fffe2f66710 (LWP 18965)] ** (totem:18955): DEBUG: Insert module media_player_keys into C module set ** (totem:18955): DEBUG: Registered extension for type 'PeasActivatable' ** (totem:18955): DEBUG: Insert module youtube into C module set ** (totem:18955): DEBUG: Registered extension for type 'PeasActivatable' ** (totem:18955): DEBUG: Insert module screenshot into C module set ** (totem:18955): DEBUG: Registered extension for type 'PeasActivatable' ** (totem:18955): DEBUG: Loading loader 'python': '/opt/gnome2/build/lib64/libpeas-1.0/loaders/libpythonloader.so' ** (totem:18955): DEBUG: Registered extension for type 'PeasPluginLoader' ** (totem:18955): DEBUG: Adding /opt/gnome2/build/lib64/totem/plugins/python as a module path for the python loader. Program received signal SIGSEGV, Segmentation fault. 0x0000003b18b00b3d in PySys_GetObject (name=0x7fffda2d1edf "path") at Python/sysmodule.c:53 53 PyObject *sd = tstate->interp->sysdict; Missing separate debuginfos, use: debuginfo-install alsa-plugins-pulseaudio-1.0.22-1.fc13.x86_64 libffi-3.0.9-1.fc13.x86_64 libuuid-2.17.2-5.fc13.x86_64 (gdb) t a a bt
+ Trace 222611
Thread 1 (Thread 0x7ffff2c01920 (LWP 18955))
(Should've mentioned: it crashed because tstate was NULL in frame #0.)
Created attachment 164759 [details] [review] Fix thread support (updated) Turns out that there were just a few GIL grabs missing from various bits of the loader. With this patch applied, Totem and its threaded plugins run fine.
Review of attachment 164759 [details] [review]: Philip, could you rebase this patch? It doesn't apply anymore already. You can put your own name and email (actually I would prefer), I don't consider myself as the author of it because my non-working patch was just a straight untested port of the old gedit one. ::: loaders/python/peas-plugin-loader-python.c @@ +400,1 @@ return TRUE; Please do not remove this empty line @@ +502,3 @@ loader->priv->init_failed = FALSE; + loader->priv->py_thread_state = PyEval_SaveThread (); Is this required? It turns out we never use that value later, so maybe we can just remove this.
Created attachment 164771 [details] [review] Fix thread support (updated again)
(In reply to comment #7) > @@ +502,3 @@ > loader->priv->init_failed = FALSE; > > + loader->priv->py_thread_state = PyEval_SaveThread (); > > Is this required? It turns out we never use that value later, so maybe we can > just remove this. Looking back at the origin gedit diff, the libpeas patch was missing the matching call to PyEval_RestoreThread(). I've added it, and everything's working nicely as far as I can tell.
Review of attachment 164771 [details] [review]: It looks okay. Feel free to push it into master.