[GNOME VFS] Re: race/deadlock/cleanup fix in gnome-vfs-method.c

--=-RTWK/8kegscUdiMLt6sc
Content-Type: text/plain
Content-Transfer-Encoding: 7bit

On Wed, 2002-07-10 at 11:28, jacob berkman wrote:
> On Tue, 2002-07-09 at 15:45, jacob berkman wrote: 
> > there was a possible race condition that i mentioned yesterday, and
> > after actually trying that patch out i found that it would deadlock
> > loading the applications: method (since that recursively tries to load
> > the file: method).
> > 
>
> here's a much-simplified version.

(sigh, last one, i "promise")

 - jacob

--=-RTWK/8kegscUdiMLt6sc
Content-Disposition: attachment; filename=gnome-vfs-method.patch
Content-Transfer-Encoding: quoted-printable
Content-Type: text/x-patch; name=gnome-vfs-method.patch; charset=ISO-8859-1

Index: gnome-vfs-method.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvs/gnome/gnome-vfs/libgnomevfs/gnome-vfs-method.c,v
retrieving revision 1.22
diff -u -r1.22 gnome-vfs-method.c
--- gnome-vfs-method.c	4 Sep 2001 22:22:58 -0000	1.22
+++ gnome-vfs-method.c	10 Jul 2002 14:31:07 -0000
@@ -44,21 +44,18 @@
 typedef struct _ModuleElement ModuleElement;
=20
 static gboolean method_already_initialized =3D FALSE;
-G_LOCK_DEFINE_STATIC (method_already_initialized);
=20
 static GHashTable *module_hash =3D NULL;
-G_LOCK_DEFINE_STATIC (module_hash);
+G_LOCK_DEFINE_STATIC (gnome_vfs_method_init);
+GStaticRecMutex module_hash_lock =3D G_STATIC_REC_MUTEX_INIT;
=20
 static GList *module_path_list =3D NULL;
-G_LOCK_DEFINE_STATIC (module_path_list);
=20
 =0C
 static gboolean
 init_hash_table (void)
 {
-	G_LOCK (module_hash);
 	module_hash =3D g_hash_table_new (g_str_hash, g_str_equal);
-	G_UNLOCK (module_hash);
=20
 	return TRUE;
 }
@@ -101,25 +98,16 @@
 init_path_list (void)
 {
 	const gchar *user_path_list;
-	gboolean retval;
=20
-	retval =3D TRUE;
-
-	G_LOCK (module_path_list);
-
-	if (module_path_list !=3D NULL) {
-		retval =3D TRUE;
-		goto end;
-	}
+	if (module_path_list !=3D NULL)
+		return TRUE;
=20
 	/* User-supplied path.  */
=20
 	user_path_list =3D getenv ("GNOME_VFS_MODULE_PATH");
 	if (user_path_list !=3D NULL) {
-		if (! install_path_list (user_path_list)) {
-			retval =3D FALSE;
-			goto end;
-		}
+		if (! install_path_list (user_path_list))
+			return FALSE;
 	}
=20
 	/* Default path.  It comes last so that users can override it.  */
@@ -127,30 +115,28 @@
 	module_path_list =3D g_list_append (module_path_list,
 					  g_strdup (GNOME_VFS_MODULE_DIR));
=20
- end:
-	G_UNLOCK (module_path_list);
-	return retval;
+	return TRUE;
 }
=20
 gboolean
 gnome_vfs_method_init (void)
 {
-	G_LOCK (method_already_initialized);
+	G_LOCK (gnome_vfs_method_init);
=20
-	if (method_already_initialized) {
-		G_UNLOCK (method_already_initialized);
-		return TRUE;
-	}
+	if (method_already_initialized)
+		goto gnome_vfs_method_init_out;
=20
 	if (! init_hash_table ())
-		return FALSE;
+		goto gnome_vfs_method_init_out;
 	if (! init_path_list ())
-		return FALSE;
+		goto gnome_vfs_method_init_out;
=20
 	method_already_initialized =3D TRUE;
-	G_UNLOCK (method_already_initialized);
=20
-	return TRUE;
+ gnome_vfs_method_init_out:
+	G_UNLOCK (gnome_vfs_method_init);
+
+	return method_already_initialized;
 }
=20
 static void
@@ -270,7 +256,7 @@
 	}
 }
=20
-static gboolean
+static ModuleElement *
 gnome_vfs_add_module_to_hash_table (const gchar *name)
 {
 	GnomeVFSMethod *method =3D NULL;
@@ -281,16 +267,16 @@
 	gid_t saved_gid;
 	const char *args;
=20
-	G_LOCK (module_hash);
+	g_static_rec_mutex_lock (&module_hash_lock);
+
 	module_element =3D g_hash_table_lookup (module_hash, name);
-	G_UNLOCK (module_hash);
=20
 	if (module_element !=3D NULL)
-		return TRUE;
+		goto add_module_out;
=20
 	module_name =3D gnome_vfs_configuration_get_module_path (name, &args);
 	if (module_name =3D=3D NULL)
-		return FALSE;
+		goto add_module_out;
=20
 	/* Set the effective UID/GID to the user UID/GID to prevent attacks to
            setuid/setgid executables.  */
@@ -309,18 +295,19 @@
 	setegid (saved_gid);
=20
 	if (method =3D=3D NULL && transform =3D=3D NULL)
-		return FALSE;
+		goto add_module_out;
=20
 	module_element =3D g_new (ModuleElement, 1);
 	module_element->name =3D g_strdup (name);
 	module_element->method =3D method;
 	module_element->transform =3D transform;
=20
-	G_LOCK (module_hash);
 	g_hash_table_insert (module_hash, module_element->name, module_element);
-	G_UNLOCK (module_hash);
=20
-	return TRUE;
+ add_module_out:
+	g_static_rec_mutex_unlock (&module_hash_lock);
+
+	return module_element;
 }
=20
 GnomeVFSMethod *
@@ -330,23 +317,8 @@
=20
 	g_return_val_if_fail (name !=3D NULL, NULL);
=20
-	G_LOCK (module_hash);
-	module_element =3D g_hash_table_lookup (module_hash, name);
-	G_UNLOCK (module_hash);
-
-	if (module_element !=3D NULL)
-		return module_element->method;
-
-	if (gnome_vfs_add_module_to_hash_table (name)) {
-		G_LOCK (module_hash);
-		module_element =3D g_hash_table_lookup (module_hash, name);
-		G_UNLOCK (module_hash);
-	=09
-		if (module_element !=3D NULL)
-			return module_element->method;
-	}
-
-	return NULL;
+	module_element =3D gnome_vfs_add_module_to_hash_table (name);
+	return module_element ? module_element->method : NULL;
 }
=20
 GnomeVFSTransform *
@@ -356,21 +328,6 @@
=20
 	g_return_val_if_fail (name !=3D NULL, NULL);
=20
-	G_LOCK (module_hash);
-	module_element =3D g_hash_table_lookup (module_hash, name);
-	G_UNLOCK (module_hash);
-
-	if (module_element !=3D NULL)
-		return module_element->transform;
-
-	if (gnome_vfs_add_module_to_hash_table (name)) {
-		G_LOCK (module_hash);
-		module_element =3D g_hash_table_lookup (module_hash, name);
-		G_UNLOCK (module_hash);
-	=09
-		if (module_element !=3D NULL)
-			return module_element->transform;
-	}
-
-	return NULL;
+	module_element =3D gnome_vfs_add_module_to_hash_table (name);
+	return module_element ? module_element->transform : NULL;
 }

--=-RTWK/8kegscUdiMLt6sc--


_______________________________________________
gnome-vfs maillist  -  [email protected]
http://lists.ximian.com/mailman/listinfo/gnome-vfs