diff --git a/generator/c.ml b/generator/c.ml index 86d3b26f8..ea69abf76 100644 --- a/generator/c.ml +++ b/generator/c.ml @@ -1704,7 +1704,7 @@ and generate_client_actions actions () = ~dll_public:true c_name style; pr "{\n"; - pr " ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g);\n"; + pr " ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock);\n"; handle_null_optargs optargs c_name; @@ -1791,7 +1791,7 @@ and generate_client_actions actions () = c_name style; pr "{\n"; - pr " ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g);\n"; + pr " ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock);\n"; handle_null_optargs optargs c_name; @@ -2137,7 +2137,7 @@ and generate_client_actions_variants () = ~handle:"g" ~prefix:"guestfs_" ~suffix:"_va" ~optarg_proto:VA c_name style; pr "{\n"; - pr " ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g);\n"; + pr " ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock);\n"; pr " struct guestfs_%s_argv optargs_s;\n" c_name; pr " struct guestfs_%s_argv *optargs = &optargs_s;\n" c_name; pr " int i;\n"; @@ -2195,7 +2195,7 @@ and generate_client_actions_variants () = ~handle:"g" ~prefix:"guestfs_" name (ret, args, []); pr "{\n"; - pr " ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g);\n"; + pr " ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock);\n"; pr " struct guestfs_%s_opts_argv optargs_s = { .bitmask = 0 };\n" name; pr " struct guestfs_%s_opts_argv *optargs = &optargs_s;\n" name; pr "\n"; diff --git a/lib/errors.c b/lib/errors.c index e3974ed85..27ab30119 100644 --- a/lib/errors.c +++ b/lib/errors.c @@ -126,7 +126,7 @@ guestfs_int_free_error_data_list (guestfs_h *g) { struct error_data *p, *next_p; - gl_lock_lock (g->error_data_list_lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->error_data_list_lock); for (p = g->error_data_list; p != NULL; p = next_p) { next_p = p->next; @@ -134,8 +134,6 @@ guestfs_int_free_error_data_list (guestfs_h *g) } g->error_data_list = NULL; - - gl_lock_unlock (g->error_data_list_lock); } /* Get thread-specific error_data struct. Create it if necessary. */ @@ -144,7 +142,7 @@ get_error_data (guestfs_h *g) { struct error_data *ret; - ret = gl_tls_get (g->error_data); + ret = pthread_getspecific (g->error_data); /* Not allocated yet for this thread, so allocate one. */ if (ret == NULL) { @@ -159,15 +157,16 @@ get_error_data (guestfs_h *g) * associated with this handle, so we can free them when the * handle is closed. */ - gl_lock_lock (g->error_data_list_lock); - ret->next = g->error_data_list; - g->error_data_list = ret; - gl_lock_unlock (g->error_data_list_lock); + { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->error_data_list_lock); + ret->next = g->error_data_list; + g->error_data_list = ret; + } /* Set the TLS to point to the struct. This is safe because we * should have acquired the handle lock. */ - gl_tls_set (g->error_data, ret); + pthread_setspecific (g->error_data, ret); } return ret; @@ -176,14 +175,14 @@ get_error_data (guestfs_h *g) const char * guestfs_last_error (guestfs_h *g) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); return get_error_data (g)->last_error; } int guestfs_last_errno (guestfs_h *g) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); return get_error_data (g)->last_errnum; } @@ -347,14 +346,14 @@ guestfs_int_perrorf (guestfs_h *g, const char *fs, ...) void guestfs_set_out_of_memory_handler (guestfs_h *g, guestfs_abort_cb cb) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); g->abort_cb = cb; } guestfs_abort_cb guestfs_get_out_of_memory_handler (guestfs_h *g) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); return g->abort_cb; } @@ -362,7 +361,7 @@ void guestfs_set_error_handler (guestfs_h *g, guestfs_error_handler_cb cb, void *data) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); struct error_data *error_data; error_data = get_error_data (g); @@ -373,7 +372,7 @@ guestfs_set_error_handler (guestfs_h *g, guestfs_error_handler_cb guestfs_get_error_handler (guestfs_h *g, void **data_rtn) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); struct error_data *error_data = get_error_data (g); if (data_rtn) *data_rtn = error_data->error_cb_data; @@ -384,7 +383,7 @@ void guestfs_push_error_handler (guestfs_h *g, guestfs_error_handler_cb cb, void *data) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); struct error_data *error_data; struct error_cb_stack *old_stack; @@ -401,7 +400,7 @@ guestfs_push_error_handler (guestfs_h *g, void guestfs_pop_error_handler (guestfs_h *g) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); struct error_data *error_data; struct error_cb_stack *next_stack; diff --git a/lib/events.c b/lib/events.c index 8005b1cc8..1037ff719 100644 --- a/lib/events.c +++ b/lib/events.c @@ -35,7 +35,7 @@ guestfs_set_event_callback (guestfs_h *g, int flags, void *opaque) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); int event_handle; if (flags != 0) { @@ -70,7 +70,7 @@ guestfs_set_event_callback (guestfs_h *g, void guestfs_delete_event_callback (guestfs_h *g, int event_handle) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); if (event_handle < 0 || event_handle >= (int) g->nr_events) return; @@ -299,7 +299,7 @@ void guestfs_set_log_message_callback (guestfs_h *g, guestfs_log_message_cb cb, void *opaque) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); replace_old_style_event_callback (g, log_message_callback_wrapper, GUESTFS_EVENT_APPLIANCE, opaque, cb); @@ -322,7 +322,7 @@ void guestfs_set_subprocess_quit_callback (guestfs_h *g, guestfs_subprocess_quit_cb cb, void *opaque) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); replace_old_style_event_callback (g, subprocess_quit_callback_wrapper, GUESTFS_EVENT_SUBPROCESS_QUIT, opaque, cb); @@ -345,7 +345,7 @@ void guestfs_set_launch_done_callback (guestfs_h *g, guestfs_launch_done_cb cb, void *opaque) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); replace_old_style_event_callback (g, launch_done_callback_wrapper, GUESTFS_EVENT_LAUNCH_DONE, opaque, cb); @@ -368,7 +368,7 @@ void guestfs_set_close_callback (guestfs_h *g, guestfs_close_cb cb, void *opaque) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); replace_old_style_event_callback (g, close_callback_wrapper, GUESTFS_EVENT_CLOSE, opaque, cb); @@ -392,7 +392,7 @@ void guestfs_set_progress_callback (guestfs_h *g, guestfs_progress_cb cb, void *opaque) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); replace_old_style_event_callback (g, progress_callback_wrapper, GUESTFS_EVENT_PROGRESS, opaque, cb); diff --git a/lib/fuse.c b/lib/fuse.c index 5f55fbc92..b4cf77c5a 100644 --- a/lib/fuse.c +++ b/lib/fuse.c @@ -47,7 +47,6 @@ #endif #include "cloexec.h" -#include "glthread/lock.h" #include "hash.h" #include "hash-pjw.h" @@ -71,7 +70,7 @@ static const struct guestfs_xattr_list *xac_lookup (guestfs_h *, const char *pat static const char *rlc_lookup (guestfs_h *, const char *pathname); /* This lock protects access to g->localmountpoint. */ -gl_lock_define_initialized (static, mount_local_lock); +static pthread_mutex_t mount_local_lock = PTHREAD_MUTEX_INITIALIZER; #define DECL_G() guestfs_h *g = fuse_get_context()->private_data #define DEBUG_CALL(fs,...) \ @@ -965,9 +964,10 @@ guestfs_impl_mount_local (guestfs_h *g, const char *localmountpoint, int fd; /* You can only mount each handle in one place in one thread. */ - gl_lock_lock (mount_local_lock); - t = g->localmountpoint; - gl_lock_unlock (mount_local_lock); + { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&mount_local_lock); + t = g->localmountpoint; + } if (t) { error (g, _("filesystem is already mounted in another thread")); return -1; @@ -1041,9 +1041,8 @@ guestfs_impl_mount_local (guestfs_h *g, const char *localmountpoint, debug (g, "%s: leaving fuse_mount_local", __func__); /* Set g->localmountpoint in the handle. */ - gl_lock_lock (mount_local_lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&mount_local_lock); g->localmountpoint = safe_strdup (g, localmountpoint); - gl_lock_unlock (mount_local_lock); return 0; } @@ -1053,9 +1052,10 @@ guestfs_impl_mount_local_run (guestfs_h *g) { int r, mounted; - gl_lock_lock (mount_local_lock); - mounted = g->localmountpoint != NULL; - gl_lock_unlock (mount_local_lock); + { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&mount_local_lock); + mounted = g->localmountpoint != NULL; + } if (!mounted) { error (g, _("you must call guestfs_mount_local first")); @@ -1084,10 +1084,9 @@ guestfs_impl_mount_local_run (guestfs_h *g) debug (g, "%s: leaving fuse_loop", __func__); guestfs_int_free_fuse (g); - gl_lock_lock (mount_local_lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&mount_local_lock); free (g->localmountpoint); g->localmountpoint = NULL; - gl_lock_unlock (mount_local_lock); /* By inspection, I found that fuse_loop only returns 0 or -1, but * don't rely on this in future. @@ -1122,12 +1121,13 @@ guestfs_impl_umount_local (guestfs_h *g, /* Make a local copy of g->localmountpoint. It could be freed from * under us by another thread, except when we are holding the lock. */ - gl_lock_lock (mount_local_lock); - if (g->localmountpoint) - localmountpoint = safe_strdup (g, g->localmountpoint); - else - localmountpoint = NULL; - gl_lock_unlock (mount_local_lock); + { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&mount_local_lock); + if (g->localmountpoint) + localmountpoint = safe_strdup (g, g->localmountpoint); + else + localmountpoint = NULL; + } if (!localmountpoint) { error (g, _("no filesystem is mounted")); diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h index 82663503c..515c4fd24 100644 --- a/lib/guestfs-internal.h +++ b/lib/guestfs-internal.h @@ -27,6 +27,7 @@ #define GUESTFS_INTERNAL_H_ #include +#include #include /* Needed on libc's different than glibc. */ #include @@ -34,6 +35,8 @@ #define PCRE2_CODE_UNIT_WIDTH 8 #include +#include + /* Minimum required version of libvirt for the libvirt backend. * * This is also checked at runtime because you can dynamically link @@ -53,8 +56,6 @@ #endif #endif -#include "glthread/lock.h" -#include "glthread/tls.h" #include "hash.h" #include "guestfs-utils.h" @@ -79,22 +80,29 @@ #define TRACE4(name, arg1, arg2, arg3, arg4) #endif +/* https://stackoverflow.com/a/1597129 */ +#define XXUNIQUE_VAR(name, line) name ## line +#define XUNIQUE_VAR(name, line) XXUNIQUE_VAR (name, line) +#define UNIQUE_VAR(name) XUNIQUE_VAR (name, __LINE__) + /* Acquire and release the per-handle lock. Note the release happens * in an __attribute__((cleanup)) handler, making it simple to write * bug-free code. */ -#define ACQUIRE_LOCK_FOR_CURRENT_SCOPE(g) \ - CLEANUP_GL_RECURSIVE_LOCK_UNLOCK gl_recursive_lock_t *_lock = &(g)->lock; \ - gl_recursive_lock_lock (*_lock) +#define ACQUIRE_LOCK_FOR_CURRENT_SCOPE(mutex) \ + CLEANUP_MUTEX_UNLOCK pthread_mutex_t *UNIQUE_VAR(_lock) = mutex; \ + do { \ + int _r = pthread_mutex_lock (UNIQUE_VAR(_lock)); \ + assert (!_r); \ + } while (0) -#define CLEANUP_GL_RECURSIVE_LOCK_UNLOCK \ - __attribute__((cleanup(guestfs_int_cleanup_gl_recursive_lock_unlock))) +#define CLEANUP_MUTEX_UNLOCK __attribute__((cleanup (cleanup_mutex_unlock))) static inline void -guestfs_int_cleanup_gl_recursive_lock_unlock (void *ptr) +cleanup_mutex_unlock (pthread_mutex_t **ptr) { - gl_recursive_lock_t *lockp = * (gl_recursive_lock_t **) ptr; - gl_recursive_lock_unlock (*lockp); + int r = pthread_mutex_unlock (*ptr); + assert (!r); } /* Default and minimum appliance memory size. */ @@ -389,7 +397,7 @@ struct guestfs_h { /* Lock acquired when entering any public guestfs_* function to * protect the handle. */ - gl_recursive_lock_define (, lock); + pthread_mutex_t lock; /**** Configuration of the handle. ****/ bool verbose; /* Debugging. */ @@ -459,12 +467,12 @@ struct guestfs_h { char *int_cachedir; /* $LIBGUESTFS_CACHEDIR or guestfs_set_cachedir or NULL */ /* Error handler, plus stack of old error handlers. */ - gl_tls_key_t error_data; + pthread_key_t error_data; /* Linked list of error_data structures allocated for this handle, * plus a mutex to protect the linked list. */ - gl_lock_define (, error_data_list_lock); + pthread_mutex_t error_data_list_lock; struct error_data *error_data_list; /* Out of memory error handler. */ diff --git a/lib/handle.c b/lib/handle.c index be6e7eea4..1ba60aaa8 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -31,8 +31,6 @@ #include #include -#include "glthread/lock.h" -#include "glthread/tls.h" #include "ignore-value.h" #include "c-ctype.h" #include "getprogname.h" @@ -44,11 +42,11 @@ static int shutdown_backend (guestfs_h *g, int check_for_errors); static void close_handles (void); -gl_lock_define_initialized (static, handles_lock); +static pthread_mutex_t handles_lock = PTHREAD_MUTEX_INITIALIZER; static guestfs_h *handles = NULL; static int atexit_handler_set = 0; -gl_lock_define_initialized (static, init_lock); +static pthread_mutex_t init_lock = PTHREAD_MUTEX_INITIALIZER; static void init_libguestfs (void) __attribute__((constructor)); @@ -61,7 +59,7 @@ static void init_libguestfs (void) __attribute__((constructor)); static void init_libguestfs (void) { - gl_lock_lock (init_lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&init_lock); #ifdef HAVE_LIBVIRT virInitialize (); @@ -69,8 +67,6 @@ init_libguestfs (void) xmlInitParser (); LIBXML_TEST_VERSION; - - gl_lock_unlock (init_lock); } guestfs_h * @@ -83,18 +79,23 @@ guestfs_h * guestfs_create_flags (unsigned flags, ...) { guestfs_h *g; + pthread_mutexattr_t attr; g = calloc (1, sizeof (*g)); if (!g) return NULL; - gl_recursive_lock_init (g->lock); - gl_lock_init (g->error_data_list_lock); + /* The per-handle lock is recursive. */ + pthread_mutexattr_init (&attr); + pthread_mutexattr_settype (&attr, PTHREAD_MUTEX_RECURSIVE); + pthread_mutex_init (&g->lock, &attr); + + pthread_mutex_init (&g->error_data_list_lock, NULL); g->state = CONFIG; g->conn = NULL; - gl_tls_key_init (g->error_data, NULL); + pthread_key_create (&g->error_data, NULL); g->abort_cb = abort; g->recovery_proc = 1; @@ -152,14 +153,13 @@ guestfs_create_flags (unsigned flags, ...) g->close_on_exit = true; /* Link the handles onto a global list. */ - gl_lock_lock (handles_lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&handles_lock); g->next = handles; handles = g; if (!atexit_handler_set) { atexit (close_handles); atexit_handler_set = 1; } - gl_lock_unlock (handles_lock); } debug (g, "create: flags = %u, handle = %p, program = %s", @@ -176,9 +176,9 @@ guestfs_create_flags (unsigned flags, ...) free (g->hv); free (g->append); guestfs_int_free_error_data_list (g); - gl_tls_key_destroy (g->error_data); - gl_lock_destroy (g->error_data_list_lock); - gl_recursive_lock_destroy (g->lock); + pthread_key_delete (g->error_data); + pthread_mutex_destroy (&g->error_data_list_lock); + pthread_mutex_destroy (&g->lock); free (g); return NULL; } @@ -329,7 +329,6 @@ guestfs_close (guestfs_h *g) { struct hv_param *hp, *hp_next; guestfs_h **gg; - int r; if (g->state == NO_HANDLE) { /* Not safe to call ANY callbacks here, so ... */ @@ -339,11 +338,10 @@ guestfs_close (guestfs_h *g) /* Remove the handle from the handles list. */ if (g->close_on_exit) { - gl_lock_lock (handles_lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&handles_lock); for (gg = &handles; *gg != g; gg = &(*gg)->next) ; *gg = g->next; - gl_lock_unlock (handles_lock); } if (g->trace) { @@ -409,22 +407,8 @@ guestfs_close (guestfs_h *g) guestfs_int_free_string_list (g->backend_settings); free (g->append); guestfs_int_free_error_data_list (g); - gl_tls_key_destroy (g->error_data); - r = glthread_recursive_lock_destroy (&g->lock); - if (r != 0) { - /* If pthread_mutex_destroy returns 16 (EBUSY), this indicates - * that the lock is held somewhere. That means a programming - * error if the main program is using threads. - */ - errno = r; - perror ("guestfs_close: g->lock"); - /* While we're debugging locks in libguestfs I want this to fail - * noisily. Remove this later since there are valid times when - * this might fail such as if the program exits during a - * libguestfs operation. - */ - abort (); - } + pthread_key_delete (g->error_data); + pthread_mutex_destroy (&g->lock); free (g); } diff --git a/lib/lpj.c b/lib/lpj.c index 37dfe2f25..4b8502a06 100644 --- a/lib/lpj.c +++ b/lib/lpj.c @@ -25,7 +25,7 @@ #include #include -#include "glthread/lock.h" +#include #include "guestfs.h" #include "guestfs-internal.h" @@ -50,7 +50,7 @@ * (Suggested by Marcelo Tosatti) */ -gl_lock_define_initialized (static, lpj_lock); +static pthread_mutex_t lpj_lock = PTHREAD_MUTEX_INITIALIZER; static int lpj = 0; static int read_lpj_from_dmesg (guestfs_h *g); static int read_lpj_from_files (guestfs_h *g); @@ -61,9 +61,9 @@ guestfs_int_get_lpj (guestfs_h *g) { int r; - gl_lock_lock (lpj_lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lpj_lock); if (lpj != 0) - goto out; + return lpj; /* Try reading lpj from these sources: * - /proc/cpuinfo [in future] @@ -73,14 +73,10 @@ guestfs_int_get_lpj (guestfs_h *g) * + /var/log/boot.msg */ r = read_lpj_from_dmesg (g); - if (r > 0) { - lpj = r; - goto out; - } - lpj = read_lpj_from_files (g); + if (r > 0) + return lpj = r; - out: - gl_lock_unlock (lpj_lock); + lpj = read_lpj_from_files (g); return lpj; } diff --git a/lib/private-data.c b/lib/private-data.c index 11acc7f97..7acf52d5f 100644 --- a/lib/private-data.c +++ b/lib/private-data.c @@ -81,7 +81,7 @@ freer (void *x) void guestfs_set_private (guestfs_h *g, const char *key, void *data) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); struct pda_entry *new_entry, *old_entry, *entry; if (g->pda == NULL) { @@ -106,7 +106,7 @@ guestfs_set_private (guestfs_h *g, const char *key, void *data) void * guestfs_get_private (guestfs_h *g, const char *key) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); if (g->pda == NULL) return NULL; /* no keys have been set */ @@ -123,7 +123,7 @@ guestfs_get_private (guestfs_h *g, const char *key) void * guestfs_first_private (guestfs_h *g, const char **key_rtn) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); if (g->pda == NULL) return NULL; @@ -144,7 +144,7 @@ guestfs_first_private (guestfs_h *g, const char **key_rtn) void * guestfs_next_private (guestfs_h *g, const char **key_rtn) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (g); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&g->lock); if (g->pda == NULL) return NULL;