Richard W.M. Jones
2018-Feb-28 15:05 UTC
[Libguestfs] [PATCH] lib: Don't abort if a signal handler calls exit(2) during a guestfs_* function.
$ virt-sparsify input output [ 0.0] Create overlay file in /tmp to protect source disk [ 0.0] Examine source disk ^C guestfs_close: g->lock: Device or resource busy Aborted (core dumped) The reason for this is because virt-sparsify calls exit(2) in the SIGINT signal handler, which causes the close_handles atexit handler to run, which calls guestfs_close. However the same handle is in the middle of a guestfs_* call, and the code in lib/handle.c catches this case and calls abort(2). (The same situation could happen from threaded code.) The solution is to ignore the case where guestfs_close is called from close_handles and the lock is held (the handle will be left open in this case, but the recovery process should reap qemu). --- lib/handle.c | 75 +++++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 24 deletions(-) diff --git a/lib/handle.c b/lib/handle.c index 449ab42a6..19caad813 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -43,6 +43,7 @@ static int shutdown_backend (guestfs_h *g, int check_for_errors); static void close_handles (void); +static void do_close (guestfs_h *g, int ignore_recursive); gl_lock_define_initialized (static, handles_lock); static guestfs_h *handles = NULL; @@ -323,15 +324,7 @@ guestfs_impl_parse_environment_list (guestfs_h *g, char * const *strings) void 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 ... */ - fprintf (stderr, _("guestfs_close: called twice on the same handle\n")); - return; - } /* Remove the handle from the handles list. */ if (g->close_on_exit) { @@ -342,6 +335,45 @@ guestfs_close (guestfs_h *g) gl_lock_unlock (handles_lock); } + do_close (g, 0); +} + +static void +do_close (guestfs_h *g, int ignore_recursive) +{ + struct hv_param *hp, *hp_next; + int r; + + if (g->state == NO_HANDLE) { + /* Not safe to call ANY callbacks here, so ... */ + fprintf (stderr, _("guestfs_close: called twice on the same handle\n")); + return; + } + + /* Destroy the handle lock first so we can detect the case where + * guestfs_close is called during another guestfs_* call on the same + * handle. This can happen if exit(3) is called from a signal + * handler or another thread during a guestfs_* call, which will + * cause the close_handles atexit handler to run, calling us here + * with ignore_recursive == 1. + */ + r = glthread_recursive_lock_destroy (&g->lock); + if (r != 0) { + if (r == EBUSY && ignore_recursive) + return; + + /* 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. + */ + abort (); + } + if (g->trace) { const char trace_msg[] = "close"; @@ -403,21 +435,6 @@ guestfs_close (guestfs_h *g) 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 (); - } free (g); } @@ -492,7 +509,17 @@ shutdown_backend (guestfs_h *g, int check_for_errors) static void close_handles (void) { - while (handles) guestfs_close (handles); + guestfs_h *g, *g_next; + + gl_lock_lock (handles_lock); + g = handles; + handles = NULL; + while (g) { + g_next = g->next; + do_close (g, 1); + g = g_next; + } + gl_lock_unlock (handles_lock); } int -- 2.13.2
Maybe Matching Threads
- [PATCH 2/5] threads: Acquire and release the lock around each public guestfs_* API.
- [PATCH v3 0/6] v2v: Add -o rhv-upload output mode.
- Re: [PATCH 2/5] threads: Acquire and release the lock around each public guestfs_* API.
- [PATCH] Improving sftp client performance
- [PATCH v3 3/5] threads: Use thread-local storage for errors.