GNOME Bugzilla – Bug 739619
Make libpeas thread-safe
Last modified: 2014-11-18 18:40:30 UTC
The main advantage of this would be to allow each PeasEngine to be used from a separate thread for performance reasons.
Created attachment 289981 [details] [review] Make the plugin loaders thread-safe This is required for making PeasEngine thread-safe. Only makes the C and Python loaders thread-safe, the Lua plugin loader will use a GRecMutex but has not been included in this patch as it has not been included into git master yet.
Created attachment 290235 [details] [review] Make the C plugin loader thread-safe
Created attachment 290236 [details] [review] Make the Python plugin loader thread-safe
Created attachment 290237 [details] [review] Document that the lua5.1 plugin loader is not thread-safe
Created attachment 290238 [details] [review] Make PeasEngine thread-safe
Created attachment 290239 [details] [review] Add an extension test that uses multiple threads This should act as a good stress test for checking that libpeas is thread-safe. This also changes testing-utils to be thread aware.
Review of attachment 290235 [details] [review]: Seems fine to me.
Review of attachment 290236 [details] [review]: hard to review this one, but if you tested it go ahead.
Review of attachment 290237 [details] [review]: I am ok with this but why lua can't? should there be a comment somewhere in the lua loader?
Review of attachment 290238 [details] [review]: See the comment ::: libpeas/peas-engine.c @@ +451,3 @@ + LoaderInfo *loader_info = &engine->priv->loaders[i]; + + if (loader_info->loader != NULL) no need for this check @@ +1379,3 @@ * of this function will return a new instance of #PeasEngine. * + * Note: this function should never be used when multiple threads are I think it would be possible to make get_default thread safe but it would be a pita
Review of attachment 290239 [details] [review]: OK
(In reply to comment #10) > Review of attachment 290238 [details] [review]: > > See the comment > > ::: libpeas/peas-engine.c > @@ +451,3 @@ > + LoaderInfo *loader_info = &engine->priv->loaders[i]; > + > + if (loader_info->loader != NULL) > > no need for this check > Will do. > @@ +1379,3 @@ > * of this function will return a new instance of #PeasEngine. > * > + * Note: this function should never be used when multiple threads are > > I think it would be possible to make get_default thread safe but it would be a > pita It would be possible if it used a GPrivate, but because get_default() doesn't return a new ref it wouldn't work without it. But really I don't think get_default() should be called from a non-main thread as it is just asking for issues and it could break existing users. For example, as part of main() the engine is created but it is only used by another thread for the rest of the program and it uses get_default() to get the originally created engine. Suddenly things no longer work because get_default() from another thread causes a crash.
(In reply to comment #9) > Review of attachment 290237 [details] [review]: > > I am ok with this but why lua can't? should there be a comment somewhere in the > lua loader? I will add a link to the issue I just created. https://github.com/pavouk/lgi/issues/92
Possible solution in LGI issue, will update the lua5.1 plugin loader as needed. Should these just be merged until a solution is found? We have a few months until freeze...
Created attachment 290813 [details] [review] Make PeasEngine thread-safe v2
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.