Created attachment 292781 [details] [review]
Fix Lua plugins to work with multiple threads

Unlock the LGI lock after init and acquire and release it
as necessary. This allows plugins to be used from multiple
threads and global plugin loader limitations.

Review of attachment 292781 [details] [review]:

See the comments.

::: loaders/lua5.1/peas-plugin-loader-lua.c
@@ +403,3 @@
     }
 
+  /* Initially the lock is taken,

why? me from the ignorance from lua, who has it?

@@ +432,3 @@
 
+  /* Make sure to take the lock before calling Lua code */
+  lua_loader->priv->lgi_enter_func (lua_loader->priv->lgi_lock);

why not a leave after the lua_close?

(In reply to comment #2)
> Review of attachment 292781 [details] [review]:
> 
> See the comments.
> 
> ::: loaders/lua5.1/peas-plugin-loader-lua.c
> @@ +403,3 @@
>      }
> 
> +  /* Initially the lock is taken,
> 
> why? me from the ignorance from lua, who has it?
> 

LGI does, because it is the lgi_lock ;)

> @@ +432,3 @@
> 
> +  /* Make sure to take the lock before calling Lua code */
> +  lua_loader->priv->lgi_enter_func (lua_loader->priv->lgi_lock);
> 
> why not a leave after the lua_close?

Because lua_close will be calling Lua code and the lock is only valid while the lua_State is valid/open. Also a callback could be running Lua code so we should block until the callback is done.

Created attachment 292795 [details] [review]
Fix Lua plugins to work with multiple threads v2

Updated comments

Review of attachment 292795 [details] [review]:

One more comment, sorry for not spotting before

::: loaders/lua5.1/peas-plugin-loader-lua.c
@@ +390,3 @@
+      lua_loader->priv->lgi_leave_func == NULL)
+    {
+      g_warning ("Failed to find 'lgi.lock', 'lgi.enter' and 'lgi.leave'");

I know it is not so clean but you should probably do individual checks
get the lock and check if it is null and then already return without checking the next one

(In reply to comment #5)
> Review of attachment 292795 [details] [review]:
> 
> One more comment, sorry for not spotting before
> 
> ::: loaders/lua5.1/peas-plugin-loader-lua.c
> @@ +390,3 @@
> +      lua_loader->priv->lgi_leave_func == NULL)
> +    {
> +      g_warning ("Failed to find 'lgi.lock', 'lgi.enter' and 'lgi.leave'");
> 
> I know it is not so clean but you should probably do individual checks
> get the lock and check if it is null and then already return without checking
> the next one

All of these fields were added at the same time, if any of them are missing either the LGI version is too old (a release hasn't been made yet) or LGI is terribly broken. I don't really think we should need to check each individually, this is going to become just a basic sanity check once an LGI release is made.

Thoughts?

Created attachment 292796 [details] [review]
Test that plugins are multi-thread safe

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.