Libguestfs - Dec 2009 - [PATCH] lib: Add thread-safety to global list of handles.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
-------------- next part --------------
>From 8bef0ef3f3a65360f9fa6ccebb62c21a8463d5fe Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones at redhat.com>
Date: Fri, 4 Dec 2009 14:35:55 +0000
Subject: [PATCH] lib: Add thread-safety to global list of handles.

This commit uses the Gnulib 'lock' module to implement a mutex on
the global list of handles which is stored by the library.

Note that Gnulib nicely avoids explicitly linking with -lpthread
unless the application program itself links to -lpthread.  Locks
are only enabled in multithreaded applications.

$ ldd src/.libs/libguestfs.so.0.217.0
	linux-vdso.so.1 =>  (0x00007fffcb7ff000)
	libc.so.6 => /lib64/libc.so.6 (0x00007f96a4e6c000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f96a544d000)

Please enter the commit message for your changes. Lines starting
---
 bootstrap     |    1 +
 m4/.gitignore |    2 ++
 src/guestfs.c |   19 ++++++++++---------
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/bootstrap b/bootstrap
index 0f8bf20..6007e59 100755
--- a/bootstrap
+++ b/bootstrap
@@ -66,6 +66,7 @@ gnumakefile
 hash
 hash-pjw
 ignore-value
+lock
 maintainer-makefile
 manywarnings
 netinet_in
diff --git a/m4/.gitignore b/m4/.gitignore
index 6b151ac..4241a4f 100644
--- a/m4/.gitignore
+++ b/m4/.gitignore
@@ -126,3 +126,5 @@ xsize.m4
 /hash.m4
 /inttostr.m4
 /putenv.m4
+/thread.m4
+/yield.m4
diff --git a/src/guestfs.c b/src/guestfs.c
index 4c94c32..7cc09ce 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -60,12 +60,14 @@
 #include <arpa/inet.h>
 #include <netinet/in.h>
 
+#include "c-ctype.h"
+#include "glthread/lock.h"
+#include "ignore-value.h"
+
 #include "guestfs.h"
 #include "guestfs-internal.h"
 #include "guestfs-internal-actions.h"
 #include "guestfs_protocol.h"
-#include "c-ctype.h"
-#include "ignore-value.h"
 
 #ifdef HAVE_GETTEXT
 #include "gettext.h"
@@ -150,6 +152,7 @@ struct guestfs_h
   int msg_next_serial;
 };
 
+gl_lock_define_initialized (static, handles_lock);
 static guestfs_h *handles = NULL;
 static int atexit_handler_set = 0;
 
@@ -217,17 +220,15 @@ guestfs_create (void)
    */
   g->msg_next_serial = 0x00123400;
 
-  /* Link the handles onto a global list.  This is the one area
-   * where the library needs to be made thread-safe. (XXX)
-   */
-  /* acquire mutex (XXX) */
+  /* Link the handles onto a global list. */
+  gl_lock_lock (handles_lock);
   g->next = handles;
   handles = g;
   if (!atexit_handler_set) {
     atexit (close_handles);
     atexit_handler_set = 1;
   }
-  /* release mutex (XXX) */
+  gl_lock_unlock (handles_lock);
 
   if (g->verbose)
     fprintf (stderr, "new guestfs handle %p\n", g);
@@ -312,7 +313,7 @@ guestfs_close (guestfs_h *g)
   /* Mark the handle as dead before freeing it. */
   g->state = NO_HANDLE;
 
-  /* acquire mutex (XXX) */
+  gl_lock_lock (handles_lock);
   if (handles == g)
     handles = g->next;
   else {
@@ -320,7 +321,7 @@ guestfs_close (guestfs_h *g)
       ;
     gg->next = g->next;
   }
-  /* release mutex (XXX) */
+  gl_lock_unlock (handles_lock);
 
   free (g->last_error);
   free (g->path);
-- 
1.6.5.2

Richard W.M. Jones wrote:
> Subject: [PATCH] lib: Add thread-safety to global list of handles.
>
> This commit uses the Gnulib 'lock' module to implement a mutex on
> the global list of handles which is stored by the library.
>
> Note that Gnulib nicely avoids explicitly linking with -lpthread
> unless the application program itself links to -lpthread.  Locks
> are only enabled in multithreaded applications.
>
> $ ldd src/.libs/libguestfs.so.0.217.0
> 	linux-vdso.so.1 =>  (0x00007fffcb7ff000)
> 	libc.so.6 => /lib64/libc.so.6 (0x00007f96a4e6c000)
> 	/lib64/ld-linux-x86-64.so.2 (0x00007f96a544d000)
>
> Please enter the commit message for your changes. Lines starting
> ---
>  bootstrap     |    1 +
>  m4/.gitignore |    2 ++
>  src/guestfs.c |   19 ++++++++++---------
Hi Rich,
That change looks fine.
However, don't you want to link with $(LIBTHREAD)?

This is from gnulib/modules/lock:

    Link:
    $(LTLIBTHREAD) when linking with libtool, $(LIBTHREAD) otherwise

which should imply you see that advice when running gnulib-tool
(albeit along with hundreds of other less-important lines)