[PATCH] Kill the backend process during cancellation.

Ben Kibbey bjk at luxsci.net
Fri Dec 26 02:23:20 CET 2014


* src/engine-gpg.c (struct engine_gpg): Add pid member.
(gpg_cancel): Send SIGTERM then SIGKILL if needed.
* src/posix-io.c (_gpgme_io_spawn): Obtain the child PID via pipe.
* src/wait-global.c (gpgme_wait_ext): Test for cancellation before FD
status.
--

During gpgme_async_cancel() or gpgme_cancel() we'd normally have to wait
for the operation to complete before the cancellation would succeed.
This can take a long time especially when doing key generation. So
after calling the cancellation functions terminate the backend process
to let gpgme_wait() return quicker.

Since gpgme uses the double-fork method to prevent zombie processes,
the child PID of the backend process needs to be obtained in the parent
process via a pipe.

This has only been implemented in the gpg backend so far.
---
 src/engine-gpg.c  | 10 ++++++++++
 src/posix-io.c    | 16 +++++++++++++++-
 src/wait-global.c | 20 ++++++++++++++++----
 3 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/src/engine-gpg.c b/src/engine-gpg.c
index 30c3bfb..ec4f491 100644
--- a/src/engine-gpg.c
+++ b/src/engine-gpg.c
@@ -27,6 +27,7 @@
 #include <string.h>
 #include <assert.h>
 #include <errno.h>
+#include <signal.h>
 #ifdef HAVE_UNISTD_H
 # include <unistd.h>
 #endif
@@ -135,6 +136,7 @@ struct engine_gpg
 
   struct gpgme_io_cbs io_cbs;
   gpgme_pinentry_mode_t pinentry_mode;
+  pid_t pid;
 };
 
 typedef struct engine_gpg *engine_gpg_t;
@@ -371,6 +373,13 @@ gpg_cancel (void *engine)
       gpg->fd_data_map = NULL;
     }
 
+  if (gpg->pid > 0 && kill (gpg->pid, 0) == 0)
+    {
+      kill (gpg->pid, SIGTERM);
+      if (kill (gpg->pid, 0) == 0)
+        kill (gpg->pid, SIGKILL);
+    }
+
   return 0;
 }
 
@@ -1389,6 +1398,7 @@ start (engine_gpg_t gpg)
   }
 
   /*_gpgme_register_term_handler ( closure, closure_value, pid );*/
+  gpg->pid = pid;
 
   rc = add_io_cb (gpg, gpg->status.fd[0], 1, status_handler, gpg,
 		  &gpg->status.tag);
diff --git a/src/posix-io.c b/src/posix-io.c
index ac823fc..835a6bd 100644
--- a/src/posix-io.c
+++ b/src/posix-io.c
@@ -376,6 +376,7 @@ _gpgme_io_spawn (const char *path, char *const argv[], unsigned int flags,
   int i;
   int status;
   int signo;
+  int pidpipe[2];
 
   TRACE_BEG1 (DEBUG_SYSIO, "_gpgme_io_spawn", path,
 	      "path=%s", path);
@@ -391,6 +392,9 @@ _gpgme_io_spawn (const char *path, char *const argv[], unsigned int flags,
     else
       TRACE_LOG3 ("fd[%i] = 0x%x -> 0x%x", i, fd_list[i].fd, fd_list[i].dup_to);
 
+  if (pipe (pidpipe) == -1)
+    return TRACE_SYSRES (-1);
+
   pid = fork ();
   if (pid == -1)
     return TRACE_SYSRES (-1);
@@ -495,7 +499,12 @@ _gpgme_io_spawn (const char *path, char *const argv[], unsigned int flags,
       if (pid == -1)
 	_exit (1);
       else
-	_exit (0);
+        {
+          close (pidpipe[0]);
+          write (pidpipe[1], &pid, sizeof(pid_t));
+          close (pidpipe[1]);
+          _exit (0);
+        }
     }
 
   TRACE_LOG1 ("waiting for child process pid=%i", pid);
@@ -503,6 +512,11 @@ _gpgme_io_spawn (const char *path, char *const argv[], unsigned int flags,
   if (status)
     return TRACE_SYSRES (-1);
 
+  close (pidpipe[1]);
+  if (read (pidpipe[0], &pid, sizeof (pid)) != sizeof (pid))
+    pid = -1;
+  close (pidpipe[0]);
+
   for (i = 0; fd_list[i].fd != -1; i++)
     {
       if (! (flags & IOSPAWN_FLAG_NOCLOSE))
diff --git a/src/wait-global.c b/src/wait-global.c
index f03775e..77518e4 100644
--- a/src/wait-global.c
+++ b/src/wait-global.c
@@ -261,6 +261,8 @@ gpgme_wait_ext (gpgme_ctx_t ctx, gpgme_error_t *status,
       struct ctx_list_item *li;
       struct fd_table fdt;
       int nr;
+      gpgme_error_t err = 0;
+      gpgme_error_t local_op_err = 0;
 
       /* Collect the active file descriptors.  */
       LOCK (ctx_list_lock);
@@ -299,13 +301,23 @@ gpgme_wait_ext (gpgme_ctx_t ctx, gpgme_error_t *status,
 	  return NULL;
 	}
 
-      for (i = 0; i < fdt.size && nr; i++)
+      LOCK (ctx->lock);
+      if (ctx->canceled)
+        err = gpg_error (GPG_ERR_CANCELED);
+      UNLOCK (ctx->lock);
+
+      if (err)
+        {
+          /* An error occured.  Close all fds in this context,
+             and signal it.  */
+          _gpgme_cancel_with_err (ctx, err, local_op_err);
+        }
+
+      for (i = 0; !err && i < fdt.size && nr; i++)
 	{
-	  if (fdt.fds[i].fd != -1 && fdt.fds[i].signaled)
+	  if (fdt.fds[i].fd != -1 && (fdt.fds[i].signaled))
 	    {
 	      gpgme_ctx_t ictx;
-	      gpgme_error_t err = 0;
-	      gpgme_error_t local_op_err = 0;
 	      struct wait_item_s *item;
 
 	      assert (nr);
-- 
2.1.4




More information about the Gnupg-devel mailing list