2007-07-16 Marcus Brinkmann <marcus@g10code.de>

* assuan/assuan-socket.c (_assuan_close): Always use close().
	* assuan/assuan.h (_gpgme_io_close): New prototype.
	(close): New macro, define as _gpgme_io_close.

gpgme/
2007-07-16  Marcus Brinkmann  <marcus@g10code.de>

	* w32-io.c (struct reader_context_s, struct writer_context_s): New
	members REFCOUNT.
	(create_reader, create_writer): Initialize C->refcount to 1.
	(destroy_reader, destroy_writer): Only destroy if C->refcount
	drops to 0.
	(find_reader, find_writer, kill_reader, kill_writer): Beautify.
	* priv-io.h (_gpgme_io_dup): New prototype.
	* posix-io.c (_gpgme_io_dup): New function. 
	* w32-io.c (_gpgme_io_dup): Likewise.
	* w32-glib-io.c (_gpgme_io_dup): Likewise.
	* engine-gpgsm.c (start): Reverting to version 2007-07-10.

ChangeLog

@@ -1,3 +1,9 @@
+2007-07-16  Marcus Brinkmann  <marcus@g10code.de>
+
+	* assuan/assuan-socket.c (_assuan_close): Always use close().
+	* assuan/assuan.h (_gpgme_io_close): New prototype.
+	(close): New macro, define as _gpgme_io_close.
+
 2007-07-13  Marcus Brinkmann  <marcus@g10code.de>
 
 	* assuan/assuan-io.c (_assuan_simple_read, _assuan_simple_write):

assuan/README.1st

@@ -30,7 +30,8 @@ updating this directory, are:
 * assuan-io.c
 ** _assuan_simple_read() and _assuan_simple_write() must always use
    read()/write() (which actually translates to _gpgme_io_read() and
-   _gpgme_io_write()).
+   _gpgme_io_write()).  _assuan_close must always() use close() (which
+   translates to _gpgme_io_close()).
 
 
  Copyright 2004, 2007 g10 Code GmbH

assuan/assuan-socket.c

@@ -45,13 +45,13 @@
 int
 _assuan_close (int fd)
 {
-#ifndef HAVE_W32_SYSTEM
+#if defined (HAVE_W32_SYSTEM) && !defined(_ASSUAN_IN_GPGME_BUILD_ASSUAN)
-  return close (fd);
-#else
   int rc = closesocket (fd);
   if (rc && WSAGetLastError () == WSAENOTSOCK)
     rc = CloseHandle (fd);
   return rc;
+#else
+  return close (fd);
 #endif
 }
 

assuan/assuan.h

@@ -63,11 +63,13 @@
 #ifdef _ASSUAN_IN_GPGME_BUILD_ASSUAN
 #include <ath.h>
 
+int _gpgme_io_close (int fd);
 int _gpgme_io_read (int fd, void *buffer, size_t count);
 int _gpgme_io_write (int fd, const void *buffer, size_t count);
 int _gpgme_io_sendmsg (int sock, const struct msghdr *msg, int flags);
 int _gpgme_io_recvmsg (int sock, struct msghdr *msg, int flags);
 
+#define close         _gpgme_io_close
 #define read          _gpgme_io_read
 #define write         _gpgme_io_write
 #define waitpid	      _gpgme_ath_waitpid

gpgme/ChangeLog

@@ -1,3 +1,17 @@
+2007-07-16  Marcus Brinkmann  <marcus@g10code.de>
+
+	* w32-io.c (struct reader_context_s, struct writer_context_s): New
+	members REFCOUNT.
+	(create_reader, create_writer): Initialize C->refcount to 1.
+	(destroy_reader, destroy_writer): Only destroy if C->refcount
+	drops to 0.
+	(find_reader, find_writer, kill_reader, kill_writer): Beautify.
+	* priv-io.h (_gpgme_io_dup): New prototype.
+	* posix-io.c (_gpgme_io_dup): New function. 
+	* w32-io.c (_gpgme_io_dup): Likewise.
+	* w32-glib-io.c (_gpgme_io_dup): Likewise.
+	* engine-gpgsm.c (start): Reverting to version 2007-07-10.
+	
 2007-07-13  Marcus Brinkmann  <marcus@g10code.de>
 
 	* data-user.c (user_read, user_write, user_seek): Set errno and

gpgme/engine-gpgsm.c

@@ -283,6 +283,8 @@ gpgsm_cancel (void *engine)
   if (!gpgsm)
     return gpg_error (GPG_ERR_INV_VALUE);
 
+  if (gpgsm->status_cb.fd != -1)
+    _gpgme_io_close (gpgsm->status_cb.fd);
   if (gpgsm->input_cb.fd != -1)
     _gpgme_io_close (gpgsm->input_cb.fd);
   if (gpgsm->output_cb.fd != -1)
@@ -842,6 +844,7 @@ status_handler (void *opaque, int fd)
               gpgsm->colon.any = 0;
               err = gpgsm->colon.fnc (gpgsm->colon.fnc_value, NULL);
             }
+ 	  _gpgme_io_close (gpgsm->status_cb.fd);
           DEBUG2 ("fd %d: OK line - final status: %s\n",
                   fd, err? gpg_strerror (err):"ok");
 	  return err;
@@ -982,15 +985,20 @@ start (engine_gpgsm_t gpgsm, const char *command)
   if (nfds < 1)
     return gpg_error (GPG_ERR_GENERAL);	/* FIXME */
 
-  /* We used to duplicate the file descriptor so that we do not
+  /* We duplicate the file descriptor, so we can close it without
-  disturb Assuan.  But this gets in the way of the Handle-to-Thread
+     disturbing assuan.  Alternatively, we could special case
-  mapping in w32-io.c, so instead we just share the file descriptor
+     status_fd and register/unregister it manually as needed, but this
-  but leave it to Assuan to close it.  */
+     increases code duplication and is more complicated as we can not
-  gpgsm->status_cb.fd = fdlist[0];
+     use the close notifications etc.  */
+
+  gpgsm->status_cb.fd = _gpgme_io_dup (fdlist[0]);
+  if (gpgsm->status_cb.fd < 0)
+    return gpg_error_from_syserror ();
 
   if (_gpgme_io_set_close_notify (gpgsm->status_cb.fd,
 				  close_notify_handler, gpgsm))
     {
+      _gpgme_io_close (gpgsm->status_cb.fd);
       gpgsm->status_cb.fd = -1;
       return gpg_error (GPG_ERR_GENERAL);
     }

gpgme/posix-io.c

@@ -494,3 +494,10 @@ _gpgme_io_sendmsg (int fd, const struct msghdr *msg, int flags)
   errno = saved_errno;
   return nwritten;
 }
+
+
+int
+_gpgme_io_dup (int fd)
+{
+  return dup (fd);
+}

gpgme/priv-io.h

@@ -65,4 +65,7 @@ int _gpgme_io_select (struct io_select_fd_s *fds, size_t nfds, int nonblock);
    line that the child process expects.  */
 int _gpgme_io_fd2str (char *buf, int buflen, int fd);
 
+/* Like dup().  */
+int _gpgme_io_dup (int fd);
+
 #endif /* IO_H */

gpgme/w32-glib-io.c

@@ -661,3 +661,10 @@ leave:
   free (pollfds_map);
   return count;
 }
+
+
+int
+_gpgme_io_dup (int fd)
+{
+  return _dup (fd);
+}

gpgme/w32-io.c

@@ -68,6 +68,8 @@ DEFINE_STATIC_LOCK (notify_table_lock);
 struct reader_context_s {
     HANDLE file_hd;
     HANDLE thread_hd;	
+    int refcount;
+
     DECLARE_LOCK (mutex);
 
     int stop_me;
@@ -96,6 +98,8 @@ DEFINE_STATIC_LOCK (reader_table_lock);
 struct writer_context_s {
     HANDLE file_hd;
     HANDLE thread_hd;	
+    int refcount;
+
     DECLARE_LOCK (mutex);
 
     int stop_me;
@@ -248,6 +252,7 @@ create_reader (HANDLE fd)
         return NULL;
 
     c->file_hd = fd;
+    c->refcount = 1;
     c->have_data_ev = CreateEvent (&sec_attr, TRUE, FALSE, NULL);
     c->have_space_ev = CreateEvent (&sec_attr, FALSE, TRUE, NULL);
     c->stopped = CreateEvent (&sec_attr, TRUE, FALSE, NULL);
@@ -294,6 +299,12 @@ static void
 destroy_reader (struct reader_context_s *c)
 {
     LOCK (c->mutex);
+    c->refcount--;
+    if (c->refcount != 0)
+      {
+	UNLOCK (c->mutex);
+	return;
+      }
     c->stop_me = 1;
     if (c->have_space_ev) 
         SetEvent (c->have_space_ev);
@@ -315,56 +326,62 @@ destroy_reader (struct reader_context_s *c)
 }
 
 
-/* 
+/* Find a reader context or create a new one.  Note that the reader
- * Find a reader context or create a new one 
+   context will last until a _gpgme_io_close.  */
- * Note that the reader context will last until a io_close.
- */
 static struct reader_context_s *
 find_reader (int fd, int start_it)
 {
-    int i;
+  struct reader_context_s *rd = NULL;
+  int i;
 
-    for (i=0; i < reader_table_size ; i++ ) {
+  LOCK (reader_table_lock);
-        if ( reader_table[i].used && reader_table[i].fd == fd )
+  for (i = 0; i < reader_table_size; i++)
-            return reader_table[i].context;
+    if (reader_table[i].used && reader_table[i].fd == fd)
-    }
+      rd = reader_table[i].context;
-    if (!start_it)
-        return NULL;
 
-    LOCK (reader_table_lock);
+  if (rd || !start_it)
-    for (i=0; i < reader_table_size; i++ ) {
+    {
-        if (!reader_table[i].used) {
+      UNLOCK (reader_table_lock);
-            reader_table[i].fd = fd;
+      return rd;
-            reader_table[i].context = create_reader (fd_to_handle (fd));
-            reader_table[i].used = 1;
-            UNLOCK (reader_table_lock);
-            return reader_table[i].context;
-        }
     }
-    UNLOCK (reader_table_lock);
+
-    return NULL;
+  for (i = 0; i < reader_table_size; i++)
+    if (!reader_table[i].used)
+      break;
+
+  if (i != reader_table_size)
+    {
+      rd = create_reader (fd_to_handle (fd));
+      reader_table[i].fd = fd;
+      reader_table[i].context = rd;
+      reader_table[i].used = 1;
+    }
+
+  UNLOCK (reader_table_lock);
+  return rd;
 }
 
 
 static void
 kill_reader (int fd)
 {
-    int i;
+  int i;
 
-    LOCK (reader_table_lock);
+  LOCK (reader_table_lock);
-    for (i=0; i < reader_table_size; i++ ) {
+  for (i = 0; i < reader_table_size; i++)
-        if (reader_table[i].used && reader_table[i].fd == fd ) {
+    {
-            destroy_reader (reader_table[i].context);
+      if (reader_table[i].used && reader_table[i].fd == fd)
-            reader_table[i].context = NULL;
+	{
-            reader_table[i].used = 0;
+	  destroy_reader (reader_table[i].context);
-            break;
+	  reader_table[i].context = NULL;
-        }
+	  reader_table[i].used = 0;
+	  break;
+	}
     }
-    UNLOCK (reader_table_lock);
+  UNLOCK (reader_table_lock);
 }
 
 
-
 int
 _gpgme_io_read ( int fd, void *buffer, size_t count )
 {
@@ -506,6 +523,7 @@ create_writer (HANDLE fd)
         return NULL;
 
     c->file_hd = fd;
+    c->refcount = 1;
     c->have_data = CreateEvent (&sec_attr, TRUE, FALSE, NULL);
     c->is_empty  = CreateEvent (&sec_attr, TRUE, TRUE, NULL);
     c->stopped = CreateEvent (&sec_attr, TRUE, FALSE, NULL);
@@ -552,6 +570,12 @@ static void
 destroy_writer (struct writer_context_s *c)
 {
     LOCK (c->mutex);
+    ctx->refcount--;
+    if (ctx->refcount != 0)
+      {
+	UNLOCK (ctx->mutex);
+	return;
+      }
     c->stop_me = 1;
     if (c->have_data) 
         SetEvent (c->have_data);
@@ -580,45 +604,54 @@ destroy_writer (struct writer_context_s *c)
 static struct writer_context_s *
 find_writer (int fd, int start_it)
 {
-    int i;
+  struct writer_context_s *wt = NULL;
+  int i;
 
-    for (i=0; i < writer_table_size ; i++ ) {
+  LOCK (writer_table_lock);
-        if ( writer_table[i].used && writer_table[i].fd == fd )
+  for (i = 0; i < writer_table_size; i++)
-            return writer_table[i].context;
+    if (writer_table[i].used && writer_table[i].fd == fd)
-    }
+      wt = writer_table[i].context;
-    if (!start_it)
-        return NULL;
 
-    LOCK (writer_table_lock);
+  if (wt || !start_it)
-    for (i=0; i < writer_table_size; i++ ) {
+    {
-        if (!writer_table[i].used) {
+      UNLOCK (writer_table_lock);
-            writer_table[i].fd = fd;
+      return wt;
-            writer_table[i].context = create_writer (fd_to_handle (fd));
-            writer_table[i].used = 1;
-            UNLOCK (writer_table_lock);
-            return writer_table[i].context;
-        }
     }
-    UNLOCK (writer_table_lock);
+
-    return NULL;
+  for (i = 0; i < writer_table_size; i++)
+    if (!writer_table[i].used)
+      break;
+
+  if (i != writer_table_size)
+    {
+      wt = create_writer (fd_to_handle (fd));
+      writer_table[i].fd = fd;
+      writer_table[i].context = wt; 
+      writer_table[i].used = 1;
+    }
+
+  UNLOCK (writer_table_lock);
+  return wt;
 }
 
 
 static void
 kill_writer (int fd)
 {
-    int i;
+  int i;
 
-    LOCK (writer_table_lock);
+  LOCK (writer_table_lock);
-    for (i=0; i < writer_table_size; i++ ) {
+  for (i = 0; i < writer_table_size; i++)
-        if (writer_table[i].used && writer_table[i].fd == fd ) {
+    {
-            destroy_writer (writer_table[i].context);
+      if (writer_table[i].used && writer_table[i].fd == fd)
-            writer_table[i].context = NULL;
+	{
-            writer_table[i].used = 0;
+	  destroy_writer (writer_table[i].context);
-            break;
+	  writer_table[i].context = NULL;
-        }
+	  writer_table[i].used = 0;
+	  break;
+	}
     }
-    UNLOCK (writer_table_lock);
+  UNLOCK (writer_table_lock);
 }
 
 
@@ -1146,6 +1179,67 @@ _gpgme_io_fd2str (char *buf, int buflen, int fd)
   return snprintf (buf, buflen, "%d", fd);
 }
 
+
+int
+_gpgme_io_dup (int fd)
+{
+  HANDLE handle = fd_to_handle (fd);
+  HANDLE new_handle = fd_to_handle (fd);
+  int i;
+  struct reader_context_s *rd_ctx;
+  struct writer_context_s *wt_ctx;
+
+  if (!DuplicateHandle (GetCurrentProcess(), handle,
+			GetCurrentProcess(), &new_handle,
+			0, FALSE, DUPLICATE_SAME_ACCESS))
+    {
+      DEBUG1 ("DuplicateHandle failed: ec=%d\n", (int) GetLastError ());
+      /* FIXME: Translate error code.  */
+      errno = EIO;
+      return -1;
+    }
+
+  rd_ctx = find_reader (fd, 1);
+  if (rd_ctx)
+    {
+      /* No need for locking, as the only races are against the reader
+	 thread itself, which doesn't touch refcount.  */
+      rd_ctx->refcount++;
+
+      LOCK (reader_table_lock);
+      for (i = 0; i < reader_table_size; i++)
+	if (!reader_table[i].used)
+	  break;
+      /* FIXME.  */
+      assert (i != reader_table_size);
+      reader_table[i].fd = handle_to_fd (new_handle);
+      reader_table[i].context = rd_ctx;
+      reader_table[i].used = 1;
+      UNLOCK (reader_table_lock);
+    }
+
+  wt_ctx = find_writer (fd, 1);
+  if (wt_ctx)
+    {
+      /* No need for locking, as the only races are against the writer
+	 thread itself, which doesn't touch refcount.  */
+      wt_ctx->refcount++;
+
+      LOCK (writer_table_lock);
+      for (i = 0; i < writer_table_size; i++)
+	if (!writer_table[i].used)
+	  break;
+      /* FIXME.  */
+      assert (i != writer_table_size);
+      writer_table[i].fd = handle_to_fd (new_handle);
+      writer_table[i].context = wt_ctx;
+      writer_table[i].used = 1;
+      UNLOCK (writer_table_lock);
+    }
+
+  return handle_to_fd (new_handle);
+}
+
 
 /* The following interface is only useful for GPGME Glib.  */