Re: GNOME keyring in Subclipse bug

Mark Phippard <markphip_at_gmail.com> writes:

> Subclipse uses the org.apache API. This issue is definitely related to the
> nuances of the code running in Eclipse. Eclipse is an OSGI environment so
> there are many layers of Java class loaders involved in loading and running
> the code. Eclipse also uses its SWT framework for the GUI. This wrappers
> GTK on Linux and I am sure that is a factor here as well.

I suppose the problem might be connected to running concurrent GLIB
event loops in two different threads. Subversion does

     loop = g_main_loop_new(NULL);
     g_main_loop_run(loop);
     ...
     g_main_loop_quit(loop);

where NULL causes g_main_loop_new to use the default GMainContext. I
suspect that is an error:

  https://tecnocode.co.uk/2014/03/27/what-is-gmaincontext/

  "Libraries should never use g_main_context_default() (or,
   equivalently, pass NULL to a GMainContext-typed parameter)"

Perhaps we should be doing something like the patch below. It can't be
precisely that because it doesn't work when I try it, the
g_main_loop_run() call hangs.

Index: subversion/libsvn_auth_gnome_keyring/gnome_keyring.c
===================================================================
--- subversion/libsvn_auth_gnome_keyring/gnome_keyring.c (revision 1640533)
+++ subversion/libsvn_auth_gnome_keyring/gnome_keyring.c (working copy)
@@ -53,6 +53,7 @@ struct gnome_keyring_baton
 {
   const char *keyring_name;
   GnomeKeyringInfo *info;
+ GMainContext *context;
   GMainLoop *loop;
 };
 
@@ -75,6 +76,9 @@ callback_destroy_data_keyring(void *data)
       key_info->info = NULL;
     }
 
+ g_main_context_pop_thread_default(key_info->context);
+ g_main_context_unref(key_info->context);
+
   return;
 }
 
@@ -151,7 +155,9 @@ get_default_keyring_name(apr_pool_t *result_pool)
   key_info.keyring_name = NULL;
 
   /* Finds default keyring. */
- key_info.loop = g_main_loop_new(NULL, FALSE);
+ key_info.context = g_main_context_new();
+ g_main_context_push_thread_default(key_info.context);
+ key_info.loop = g_main_loop_new(key_info.context, FALSE);
   gnome_keyring_get_default_keyring(callback_default_keyring, &key_info, NULL);
   g_main_loop_run(key_info.loop);
 
@@ -177,7 +183,9 @@ check_keyring_is_locked(const char *keyring_name)
   key_info.keyring_name = NULL;
 
   /* Get details about the default keyring. */
- key_info.loop = g_main_loop_new(NULL, FALSE);
+ key_info.context = g_main_context_new();
+ g_main_context_push_thread_default(key_info.context);
+ key_info.loop = g_main_loop_new(key_info.context, FALSE);
   gnome_keyring_get_info(keyring_name, callback_get_info_keyring, &key_info,
                          NULL);
   g_main_loop_run(key_info.loop);
@@ -208,7 +216,9 @@ unlock_gnome_keyring(const char *keyring_name,
   key_info.keyring_name = NULL;
 
   /* Get details about the default keyring. */
- key_info.loop = g_main_loop_new(NULL, FALSE);
+ key_info.context = g_main_context_new();
+ g_main_context_push_thread_default(key_info.context);
+ key_info.loop = g_main_loop_new(key_info.context, FALSE);
   gnome_keyring_get_info(keyring_name, callback_get_info_keyring,
                          &key_info, NULL);
   g_main_loop_run(key_info.loop);
@@ -220,7 +230,9 @@ unlock_gnome_keyring(const char *keyring_name,
     }
   else
     {
- key_info.loop = g_main_loop_new(NULL, FALSE);
+ key_info.context = g_main_context_new();
+ g_main_context_push_thread_default(key_info.context);
+ key_info.loop = g_main_loop_new(key_info.context, FALSE);
       gnome_keyring_unlock(keyring_name, keyring_password,
                            callback_done, &key_info, NULL);
       g_main_loop_run(key_info.loop);

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*