doc/

2002-05-09  Marcus Brinkmann  <marcus@g10code.de>

	* gpgme.texi (Overview): Replace note about thread-safeness.
	(Multi Threading): New section.

gpgme/
2002-05-08  Marcus Brinkmann  <marcus@g10code.de>

	* w32-util.c: New static variable GET_PATH_LOCK.
	(_gpgme_get_gpg_path): Remove superfluous NULL initializer.
	Take lock while determining path.
	(_gpgme_get_gpgsm_path): Likewise.
	* version.c (do_subsystem_inits): Set DONE to 1 after
	initialization.
	(gpgme_get_engine_info): New variable ENGINE_INFO_LOCK.  Take lock
	while determining engine info.
	* rungpg.c (_gpgme_gpg_get_version): New variable
	GPG_VERSION_LOCK.  Take the lock while determining the program
	version.
	* posix-io.c: Include "sema.h".
	(_gpgme_io_spawn): New variable FIXED_SIGNALS_LOCK.  Take the lock
	while fixing the signals.
	(_gpgme_io_select): Make READFDS and WRITEFDS non-static.
	* key.c: Include "sema.h".  New globals KEY_CACHE_LOCK and
	KEY_REF_LOCK.
	(capabilities_to_string): Make STRINGS very const.
	(_gpgme_key_cache_add): Lock the key cache.
	(_gpgme_key_cache_get): Likewise.
	(gpgme_key_ref, gpgme_key_release): Lock the key_ref_lock.
	* import.c (append_xml_impinfo): Make IMPORTED_FIELDS and
	IMPORT_RES_FIELDS very const.  Make FIELD and FIELD_NAME a litle
	const.
	* engine.c (_gpgme_engine_get_info): New variable
	ENGINE_INFO_LOCK.  Take lock while determining engine info.
	* engine-gpgsm.c: Include "sema.h".
	(_gpgme_gpgsm_get_version): New variable GPGSM_VERSION_LOCK.  Take
	lock while getting program version.

TODO

@@ -8,8 +8,6 @@ Hey Emacs, this is -*- outline -*- mode!
 * Allow to use GTK's main loop instead of the select stuff in
   wait.c
 
-* add locking to the key cache?
-
 * cleanup the namespace - we use log_* assuan_* ascii_* mutex_*
   But those are only used internally.  Some linker tricks should make
   it possible to hide them from the user (didn't work last time, try
@@ -30,6 +28,7 @@ Hey Emacs, this is -*- outline -*- mode!
 *** For pipemode, make sure to release the pipemode callback data object.
 
 * Operations
+** gpgme_wait needs to be made thread safe!!!
 ** Export status handler need much more work.
 ** Import should return a useful error when one happened.
 ** Genkey should return something more useful than General_Error.

doc/ChangeLog

@@ -1,3 +1,8 @@
+2002-05-09  Marcus Brinkmann  <marcus@g10code.de>
+
+	* gpgme.texi (Overview): Replace note about thread-safeness.
+	(Multi Threading): New section.
+
 2002-05-03  Werner Koch  <wk@gnupg.org>
 
 	* gpgme.texi (Manipulating Data Buffers): Changed some data types

doc/gpgme.texi

@@ -104,6 +104,7 @@ Preparation
 * Header::                        What header file you need to include.
 * Building the Source::           Compiler options to be used.
 * Library Version Check::         Getting and verifying the library version.
+* Multi Threading::               How GPGME can be used in an MT environment.
 
 Protocols and Engines
 
@@ -278,11 +279,9 @@ including listing keys, querying their attributes, generating,
 importing, exporting and deleting keys, and acquiring information
 about the trust path.
 
-@cindex thread-safeness
+With some precautions, @acronym{GPGME} can be used in a multi-threaded
-@cindex multi-threading
+environment, although it is not completely thread safe and thus needs
-@strong{Caution:} The @acronym{GPGME} library is not thread-safe.  It
+the support of the application.
-will be to some extent in the future, but currently great care has to
-be taken if @acronym{GPGME} is used in a multi-threaded environment.
 
 
 @node Preparation
@@ -298,6 +297,7 @@ of the library are verified.
 * Header::                        What header file you need to include.
 * Building the Source::           Compiler options to be used.
 * Library Version Check::         Getting and verifying the library version.
+* Multi Threading::               How GPGME can be used in an MT environment.
 @end menu
 
 
@@ -402,6 +402,81 @@ features are provided by the installed version of the library.
 @end deftypefun
 
 
+@node Multi Threading
+@section Multi Threading
+@cindex thread-safeness
+@cindex multi-threading
+
+The @acronym{GPGME} library is not entirely thread-safe, but it can
+still be used in a multi-threaded environment if some care is taken.
+If the following requirements are met, there should be no race
+conditions to worry about:
+
+@itemize @bullet
+@item
+The function @code{gpgme_check_version} must be called before any
+other function in the library, because it initializes the locking
+subsystem in @acronym{GPGME}.  To achieve this in all generality, it
+is necessary to synchronize the call to this function with all other
+calls to functions in the library, using the synchronization
+mechanisms available in your thread library.  Otherwise, specific
+compiler or CPU memory cache optimizations could lead to the situation
+where a thread is started and uses @acronym{GPGME} before the effects
+of the initialization are visible for this thread.  It doesn't even
+suffice to call @code{gpgme_check_version} before creating this other
+thread@footnote{In SMP systems the new thread could be started on
+another CPU before the effects of the initialization are seen by that
+CPU's memory cache.  Not doing proper synchronization here leads to
+the same problems the double-checked locking idiom has.  You might
+find that if you don't do proper synchronization, it still works in
+most configurations.  Don't let this fool you.  Someday it might lead
+to subtle bugs when someone tries it on a DEC Alpha or an SMP
+machine.}.
+
+For example, if you are using POSIX threads, each thread that wants to
+call functions in @acronym{GPGME} could call the following function
+before any function in the library:
+
+@example
+#include <pthread.h>
+
+void
+initialize_gpgme (void)
+{
+  static int gpgme_init;
+  static pthread_mutext_t gpgme_init_lock = PTHREAD_MUTEX_INITIALIZER;
+
+  pthread_mutex_lock (&gpgme_init_lock);
+  if (!gpgme_init)
+    {
+      gpgme_check_version ();
+      gpgme_init = 1;
+    }
+  pthread_mutex_unlock (&gpgme_init_lock);
+}
+@end example
+
+@item
+Any @code{GpgmeData}, @code{GpgmeCtx} and @code{GpgmeRecipients}
+object must only be accessed by one thread at a time.  If multiple
+threads want to deal with the same object, the caller has to make sure
+that operations on this object are fully synchronized.
+
+@item
+Only one thread is allowed to call @code{gpgme_wait} at a time.  If
+multiple threads call this function, the caller must make sure that
+all invocations are fully synchronized.
+
+@item
+Unfortunately, the last rule implies that all calls to one of the
+following functions have to be fully synchronized together with
+@code{gpgme_wait}, as they call @code{gpgme_wait} internally: All
+functions @code{gpgme_op_FOO} that have a corresponding
+@code{gpgme_op_FOO_start} function, @code{gpgme_op_keylist_next},
+@code{gpgme_op_trustlist_next}.
+@end itemize
+
+
 @node Protocols and Engines
 @chapter Protocols and Engines
 @cindex protocol

gpgme/ChangeLog

@@ -1,3 +1,35 @@
+2002-05-08  Marcus Brinkmann  <marcus@g10code.de>
+
+	* w32-util.c: New static variable GET_PATH_LOCK.
+	(_gpgme_get_gpg_path): Remove superfluous NULL initializer.
+	Take lock while determining path.
+	(_gpgme_get_gpgsm_path): Likewise.
+	* version.c (do_subsystem_inits): Set DONE to 1 after
+	initialization.
+	(gpgme_get_engine_info): New variable ENGINE_INFO_LOCK.  Take lock
+	while determining engine info.
+	* rungpg.c (_gpgme_gpg_get_version): New variable
+	GPG_VERSION_LOCK.  Take the lock while determining the program
+	version.
+	* posix-io.c: Include "sema.h".
+	(_gpgme_io_spawn): New variable FIXED_SIGNALS_LOCK.  Take the lock
+	while fixing the signals.
+	(_gpgme_io_select): Make READFDS and WRITEFDS non-static.
+	* key.c: Include "sema.h".  New globals KEY_CACHE_LOCK and
+	KEY_REF_LOCK.
+	(capabilities_to_string): Make STRINGS very const.
+	(_gpgme_key_cache_add): Lock the key cache.
+	(_gpgme_key_cache_get): Likewise.
+	(gpgme_key_ref, gpgme_key_release): Lock the key_ref_lock.
+	* import.c (append_xml_impinfo): Make IMPORTED_FIELDS and
+	IMPORT_RES_FIELDS very const.  Make FIELD and FIELD_NAME a litle
+	const.
+	* engine.c (_gpgme_engine_get_info): New variable
+	ENGINE_INFO_LOCK.  Take lock while determining engine info.
+	* engine-gpgsm.c: Include "sema.h".
+	(_gpgme_gpgsm_get_version): New variable GPGSM_VERSION_LOCK.  Take
+	lock while getting program version.
+
 2002-05-08  Marcus Brinkmann  <marcus@g10code.de>
 
 	* debug.h: New file.

gpgme/engine-gpgsm.c

@@ -48,6 +48,7 @@
 #include "wait.h"
 #include "io.h"
 #include "key.h"
+#include "sema.h"
 
 #include "engine-gpgsm.h"
 
@@ -99,10 +100,12 @@ const char *
 _gpgme_gpgsm_get_version (void)
 {
   static const char *gpgsm_version;
+  DEFINE_STATIC_LOCK (gpgsm_version_lock);
 
-  /* FIXME: Locking.  */
+  LOCK (gpgsm_version_lock);
   if (!gpgsm_version)
     gpgsm_version = _gpgme_get_program_version (_gpgme_get_gpgsm_path ());
+  UNLOCK (gpgsm_version_lock);
 
   return gpgsm_version;
 }

gpgme/engine.c

@@ -36,6 +36,7 @@
 #include "rungpg.h"
 #include "engine-gpgsm.h"
 
+
 struct engine_object_s
   {
     GpgmeProtocol protocol;
@@ -50,6 +51,7 @@ struct engine_object_s
       } engine;
 };
 
+
 struct reap_s
 {
   struct reap_s *next;
@@ -61,6 +63,7 @@ struct reap_s
 static struct reap_s *reap_list;
 DEFINE_STATIC_LOCK (reap_list_lock);
 
+
 /* Get the path of the engine for PROTOCOL.  */
 const char *
 _gpgme_engine_get_path (GpgmeProtocol proto)
@@ -76,6 +79,7 @@ _gpgme_engine_get_path (GpgmeProtocol proto)
     }
 }
 
+
 /* Get the version number of the engine for PROTOCOL.  */
 const char *
 _gpgme_engine_get_version (GpgmeProtocol proto)
@@ -91,6 +95,7 @@ _gpgme_engine_get_version (GpgmeProtocol proto)
     }
 }
 
+
 GpgmeError
 gpgme_engine_check_version (GpgmeProtocol proto)
 {
@@ -105,6 +110,7 @@ gpgme_engine_check_version (GpgmeProtocol proto)
     }
 }
 
+
 const char *
 _gpgme_engine_get_info (GpgmeProtocol proto)
 {
@@ -115,36 +121,35 @@ _gpgme_engine_get_info (GpgmeProtocol proto)
     " </engine>\n";
   static const char *const strproto[3] = { "OpenPGP", "CMS", NULL };
   static const char *engine_info[3];  /* FIXME: MAX_PROTO + 1*/
-  const char *path;
+  DEFINE_STATIC_LOCK (engine_info_lock);
-  const char *version;
-  char *info;
 
   if (proto > 2 /* FIXME MAX_PROTO */ || !strproto[proto])
     return NULL;
 
-  /* FIXME: Make sure that only one instance does run.  */
+  LOCK (engine_info_lock);
-  if (engine_info[proto])
+  if (!engine_info[proto])
-    return engine_info[proto];
+    {
-
+      const char *path = _gpgme_engine_get_path (proto);
-  path = _gpgme_engine_get_path (proto);
+      const char *version = _gpgme_engine_get_version (proto);
-  version = _gpgme_engine_get_version (proto);
-
-  if (!path || !version)
-    return NULL;
-
-  info = xtrymalloc (strlen(fmt) + strlen(strproto[proto]) + strlen(path)
-                     + strlen (version) + 1);
-  if (!info)
-    info = " <engine>\n"
-      "  <error>Out of core</error>\n"
-      " </engine>";
-  else
-    sprintf (info, fmt, strproto[proto], version, path);
-  engine_info[proto] = info;
 
+      if (path && version)
+	{
+	  char *info = xtrymalloc (strlen (fmt) + strlen (strproto[proto])
+				   + strlen (path) + strlen (version) + 1);
+	  if (!info)
+	    info = " <engine>\n"
+	      "  <error>Out of core</error>\n"
+	      " </engine>";
+	  else
+	    sprintf (info, fmt, strproto[proto], version, path);
+	  engine_info[proto] = info;
+	}
+    }
+  UNLOCK (engine_info_lock);
   return engine_info[proto];
 }
 
+
 GpgmeError
 _gpgme_engine_new (GpgmeProtocol proto, EngineObject *r_engine)
 {
@@ -193,6 +198,7 @@ _gpgme_engine_new (GpgmeProtocol proto, EngineObject *r_engine)
   return err;
 }
 
+
 void
 _gpgme_engine_release (EngineObject engine)
 {
@@ -561,6 +567,7 @@ _gpgme_engine_start (EngineObject engine, void *opaque)
   return 0;
 }
 
+
 void
 _gpgme_engine_add_child_to_reap_list (void *buf, int buflen, pid_t pid)
 {
@@ -572,10 +579,10 @@ _gpgme_engine_add_child_to_reap_list (void *buf, int buflen, pid_t pid)
   memset (child, 0, sizeof *child);
   child->pid = pid;
   child->entered = time (NULL);
-  LOCK(reap_list_lock);
+  LOCK (reap_list_lock);
   child->next = reap_list;
   reap_list = child;
-  UNLOCK(reap_list_lock);
+  UNLOCK (reap_list_lock);
 }
 
 static void

gpgme/gpgme.h

@@ -43,7 +43,7 @@ extern "C" {
    AM_PATH_GPGME macro) check that this header matches the installed
    library.  Warning: Do not edit the next line.  configure will do
    that for you!  */
-#define GPGME_VERSION "0.3.6"
+#define GPGME_VERSION "0.3.7-cvs"
 
 
 /* The opaque data types used by GPGME.  */

gpgme/import.c

@@ -53,14 +53,14 @@ static void
 append_xml_impinfo (GpgmeData *rdh, GpgStatusCode code, char *args)
 {
 #define MAX_IMPORTED_FIELDS 14
-  static char *imported_fields[MAX_IMPORTED_FIELDS]
+  static const char *const imported_fields[MAX_IMPORTED_FIELDS]
     = { "keyid", "username", 0 };
-  static char *import_res_fields[MAX_IMPORTED_FIELDS]
+  static const char *const import_res_fields[MAX_IMPORTED_FIELDS]
     = { "count", "no_user_id", "imported", "imported_rsa",
 	"unchanged", "n_uids", "n_subk", "n_sigs", "s_sigsn_revoc",
 	"sec_read", "sec_imported", "sec_dups", "skipped_new", 0 };
-  char *field[MAX_IMPORTED_FIELDS];
+  const char *field[MAX_IMPORTED_FIELDS];
-  char **field_name = 0;
+  const char *const *field_name = 0;
   GpgmeData dh;
   int i;
 

gpgme/posix-io.c

@@ -36,6 +36,7 @@
 
 #include "util.h"
 #include "io.h"
+#include "sema.h"
 
 static struct
 {
@@ -149,10 +150,12 @@ _gpgme_io_spawn (const char *path, char **argv,
 		 struct spawn_fd_item_s *fd_child_list,
 		 struct spawn_fd_item_s *fd_parent_list)
 {
-  static volatile int fixed_signals;
+  static int fixed_signals;
+  DEFINE_STATIC_LOCK (fixed_signals_lock);
   pid_t pid;
   int i;
 
+  LOCK (fixed_signals_lock);
   if (!fixed_signals)
     { 
       struct sigaction act;
@@ -166,8 +169,8 @@ _gpgme_io_spawn (const char *path, char **argv,
 	  sigaction (SIGPIPE, &act, NULL);
         }
       fixed_signals = 1;
-      /* XXX: This is not really MT safe.  */
     }
+  UNLOCK (fixed_signals_lock);
 
   pid = fork ();
   if (pid == -1) 
@@ -285,8 +288,8 @@ _gpgme_io_kill (int pid, int hard)
 int
 _gpgme_io_select (struct io_select_fd_s *fds, size_t nfds)
 {
-  static fd_set readfds;
+  fd_set readfds;
-  static fd_set writefds;
+  fd_set writefds;
   int any, i, max_fd, n, count;
   struct timeval timeout = { 1, 0 }; /* Use a 1s timeout.  */
   void *dbg_help = NULL;
@@ -314,7 +317,7 @@ _gpgme_io_select (struct io_select_fd_s *fds, size_t nfds)
         }
       else if (fds[i].for_write)
 	{
-	  assert (!FD_ISSET ( fds[i].fd, &writefds));
+	  assert (!FD_ISSET (fds[i].fd, &writefds));
 	  FD_SET (fds[i].fd, &writefds);
 	  if (fds[i].fd > max_fd)
 	    max_fd = fds[i].fd;

gpgme/rungpg.c

@@ -167,11 +167,12 @@ const char *
 _gpgme_gpg_get_version (void)
 {
   static const char *gpg_version;
+  DEFINE_STATIC_LOCK (gpg_version_lock);
 
-  /* FIXME: Locking.  */
+  LOCK (gpg_version_lock);
   if (!gpg_version)
     gpg_version = _gpgme_get_program_version (_gpgme_get_gpg_path ());
-
+  UNLOCK (gpg_version_lock);
   return gpg_version;
 }
 

gpgme/version.c

@@ -43,6 +43,7 @@ do_subsystem_inits (void)
     return;
   _gpgme_sema_subsystem_init ();
   _gpgme_key_cache_init ();
+  done = 1;
 }
 
 static const char*
@@ -148,45 +149,50 @@ const char *
 gpgme_get_engine_info ()
 {
   static const char *engine_info;
-  const char *openpgp_info = _gpgme_engine_get_info (GPGME_PROTOCOL_OpenPGP);
+  DEFINE_STATIC_LOCK (engine_info_lock);
-  const char *cms_info = _gpgme_engine_get_info (GPGME_PROTOCOL_CMS);
-  char *info;
 
-  /* FIXME: Make sure that only one instance does run.  */
+  LOCK (engine_info_lock);
-  if (engine_info)
+  if (!engine_info)
-    return engine_info;
-
-  if (!openpgp_info && !cms_info)
-    info = "<EngineInfo>\n</EngineInfo>\n";
-  else if (!openpgp_info || !cms_info)
     {
-      const char *fmt = "<EngineInfo>\n"
+      const char *openpgp_info = _gpgme_engine_get_info (GPGME_PROTOCOL_OpenPGP);
-        "%s"
+      const char *cms_info = _gpgme_engine_get_info (GPGME_PROTOCOL_CMS);
-        "</EngineInfo>\n";
+      char *info;
 
-      info = xtrymalloc (strlen(fmt) + strlen(openpgp_info
+      if (!openpgp_info && !cms_info)
-                                              ? openpgp_info : cms_info) + 1);
+	info = "<EngineInfo>\n</EngineInfo>\n";
-      if (info)
+      else if (!openpgp_info || !cms_info)
-        sprintf (info, fmt, openpgp_info ? openpgp_info : cms_info);
+	{
+	  const char *fmt = "<EngineInfo>\n"
+	    "%s"
+	    "</EngineInfo>\n";
+
+	  info = xtrymalloc (strlen (fmt)
+			     + strlen (openpgp_info
+				      ? openpgp_info : cms_info) + 1);
+	  if (info)
+	    sprintf (info, fmt, openpgp_info ? openpgp_info : cms_info);
+	}
+      else
+	{
+	  const char *fmt = "<EngineInfo>\n"
+	    "%s%s"
+	    "</EngineInfo>\n";
+	  info = xtrymalloc (strlen (fmt) + strlen (openpgp_info)
+			     + strlen (cms_info) + 1);
+	  if (info)
+	    sprintf (info, fmt, openpgp_info, cms_info);
+	}
+      if (!info)
+	info = "<EngineInfo>\n"
+	  "  <error>Out of core</error>\n"
+	  "</EngineInfo>\n";
+      engine_info = info;
     }
-  else
+  UNLOCK (engine_info_lock);
-    {
-      const char *fmt = "<EngineInfo>\n"
-        "%s%s"
-        "</EngineInfo>\n";
-      info = xtrymalloc (strlen(fmt) + strlen(openpgp_info)
-                         + strlen (cms_info) + 1);
-      if (info)
-        sprintf (info, fmt, openpgp_info, cms_info);
-    }
-  if (!info)
-    info = "<EngineInfo>\n"
-      "  <error>Out of core</error>\n"
-      "</EngineInfo>\n";
-  engine_info = info;
   return engine_info;
 }
 
+
 /**
  * gpgme_check_engine:
  * 

gpgme/w32-util.c

@@ -37,6 +37,8 @@
 
 #include "util.h"
 
+DEFINE_STATIC_LOCK (get_path_lock);
+
 /* Return a string from the Win32 Registry or NULL in case of error.
   Caller must release the return value.  A NULL for root is an alias
   for HKEY_CURRENT_USER.  */
@@ -112,27 +114,31 @@ find_program_in_registry (const char *name)
 const char *
 _gpgme_get_gpg_path (void)
 {
-  static char *gpg_program = NULL;
+  static char *gpg_program;
-    
+
+  LOCK (get_path_lock);
   if (!gpg_program)
     gpg_program = find_program_in_registry ("gpgProgram");
 #ifdef GPG_PATH
   if (!gpg_program)
     gpg_program = GPG_PATH;
 #endif
+  UNLOCK (get_path_lock);
   return gpg_program;
 }
 
 const char *
 _gpgme_get_gpgsm_path (void)
 {
-  static char *gpgsm_program = NULL;
+  static char *gpgsm_program;
-    
+
+  LOCK (get_path_lock);
   if (!gpgsm_program)
     gpgsm_program = find_program_in_registry ("gpgsmProgram");
 #ifdef GPGSM_PATH
   if (!gpgsm_program)
     gpgsm_program = GPGSM_PATH;
 #endif
+  UNLOCK (get_path_lock);
   return gpgsm_program;
 }