[Libguestfs] [PATCH] lib: Don't abort if a signal handler calls exit(2) during a guestfs_* function.

[Richard W.M. Jones]

$ 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