diff --git a/ChangeLog b/ChangeLog index e97bff37..6a340411 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2007-07-16 Marcus Brinkmann + + * 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 * assuan/assuan-io.c (_assuan_simple_read, _assuan_simple_write): diff --git a/assuan/README.1st b/assuan/README.1st index 8e805c39..46f1bb05 100644 --- a/assuan/README.1st +++ b/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 diff --git a/assuan/assuan-socket.c b/assuan/assuan-socket.c index 196c4f6f..5566fdea 100644 --- a/assuan/assuan-socket.c +++ b/assuan/assuan-socket.c @@ -45,13 +45,13 @@ int _assuan_close (int fd) { -#ifndef HAVE_W32_SYSTEM - return close (fd); -#else +#if defined (HAVE_W32_SYSTEM) && !defined(_ASSUAN_IN_GPGME_BUILD_ASSUAN) int rc = closesocket (fd); if (rc && WSAGetLastError () == WSAENOTSOCK) rc = CloseHandle (fd); return rc; +#else + return close (fd); #endif } diff --git a/assuan/assuan.h b/assuan/assuan.h index 33f14cf0..fd807959 100644 --- a/assuan/assuan.h +++ b/assuan/assuan.h @@ -63,11 +63,13 @@ #ifdef _ASSUAN_IN_GPGME_BUILD_ASSUAN #include +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 diff --git a/gpgme/ChangeLog b/gpgme/ChangeLog index fb00f9ce..441daf7d 100644 --- a/gpgme/ChangeLog +++ b/gpgme/ChangeLog @@ -1,3 +1,17 @@ +2007-07-16 Marcus Brinkmann + + * 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 * data-user.c (user_read, user_write, user_seek): Set errno and diff --git a/gpgme/engine-gpgsm.c b/gpgme/engine-gpgsm.c index 4e3f1e70..9babac54 100644 --- a/gpgme/engine-gpgsm.c +++ b/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 - disturb Assuan. But this gets in the way of the Handle-to-Thread - mapping in w32-io.c, so instead we just share the file descriptor - but leave it to Assuan to close it. */ - gpgsm->status_cb.fd = fdlist[0]; + /* We duplicate the file descriptor, so we can close it without + disturbing assuan. Alternatively, we could special case + status_fd and register/unregister it manually as needed, but this + increases code duplication and is more complicated as we can not + 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); } diff --git a/gpgme/posix-io.c b/gpgme/posix-io.c index 7d7be1e5..c44879db 100644 --- a/gpgme/posix-io.c +++ b/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); +} diff --git a/gpgme/priv-io.h b/gpgme/priv-io.h index 25d14dcb..3099a49f 100644 --- a/gpgme/priv-io.h +++ b/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 */ diff --git a/gpgme/w32-glib-io.c b/gpgme/w32-glib-io.c index 8f56eee7..813e807f 100644 --- a/gpgme/w32-glib-io.c +++ b/gpgme/w32-glib-io.c @@ -661,3 +661,10 @@ leave: free (pollfds_map); return count; } + + +int +_gpgme_io_dup (int fd) +{ + return _dup (fd); +} diff --git a/gpgme/w32-io.c b/gpgme/w32-io.c index e2a464dd..13ce15b8 100644 --- a/gpgme/w32-io.c +++ b/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 context will last until a io_close. - */ +/* Find a reader context or create a new one. Note that the reader + context will last until a _gpgme_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++ ) { - if ( reader_table[i].used && reader_table[i].fd == fd ) - return reader_table[i].context; - } - if (!start_it) - return NULL; + LOCK (reader_table_lock); + for (i = 0; i < reader_table_size; i++) + if (reader_table[i].used && reader_table[i].fd == fd) + rd = reader_table[i].context; - LOCK (reader_table_lock); - for (i=0; i < reader_table_size; i++ ) { - if (!reader_table[i].used) { - reader_table[i].fd = fd; - reader_table[i].context = create_reader (fd_to_handle (fd)); - reader_table[i].used = 1; - UNLOCK (reader_table_lock); - return reader_table[i].context; - } + if (rd || !start_it) + { + UNLOCK (reader_table_lock); + return rd; } - 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); - for (i=0; i < reader_table_size; i++ ) { - if (reader_table[i].used && reader_table[i].fd == fd ) { - destroy_reader (reader_table[i].context); - reader_table[i].context = NULL; - reader_table[i].used = 0; - break; - } + LOCK (reader_table_lock); + for (i = 0; i < reader_table_size; i++) + { + if (reader_table[i].used && reader_table[i].fd == fd) + { + destroy_reader (reader_table[i].context); + 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++ ) { - if ( writer_table[i].used && writer_table[i].fd == fd ) - return writer_table[i].context; - } - if (!start_it) - return NULL; + LOCK (writer_table_lock); + for (i = 0; i < writer_table_size; i++) + if (writer_table[i].used && writer_table[i].fd == fd) + wt = writer_table[i].context; - LOCK (writer_table_lock); - for (i=0; i < writer_table_size; i++ ) { - if (!writer_table[i].used) { - writer_table[i].fd = fd; - writer_table[i].context = create_writer (fd_to_handle (fd)); - writer_table[i].used = 1; - UNLOCK (writer_table_lock); - return writer_table[i].context; - } + if (wt || !start_it) + { + UNLOCK (writer_table_lock); + return wt; } - 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); - for (i=0; i < writer_table_size; i++ ) { - if (writer_table[i].used && writer_table[i].fd == fd ) { - destroy_writer (writer_table[i].context); - writer_table[i].context = NULL; - writer_table[i].used = 0; - break; - } + LOCK (writer_table_lock); + for (i = 0; i < writer_table_size; i++) + { + if (writer_table[i].used && writer_table[i].fd == fd) + { + destroy_writer (writer_table[i].context); + 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. */