* command.c (cmd_updatestartuptty): New.

* gpg-agent.c: New option --write-env-file.

* gpg-agent.c (handle_connections): Make sure that the signals we
are handling are not blocked.Block signals while creating new
threads.

* estream.c: Use HAVE_CONFIG_H and not USE_CONFIG_H!
(es_func_fd_read, es_func_fd_write): Protect against EINTR.

* gpg-agent.texi (Agent UPDATESTARTUPTTY): New.

* scdaemon.c (handle_connections): Make sure that the signals we
are handling are not blocked.Block signals while creating new
threads.
(handle_connections): Include the file descriptor into the name of
the thread.

7 files changed, 202 insertions, 64 deletions

diff --git a/scd/ChangeLog b/scd/ChangeLog
index 136ed5618..da433e2f8 100644
--- a/scd/ChangeLog
+++ b/scd/ChangeLog
@@ -1,3 +1,32 @@
+2005-06-03  Werner Koch  <wk@g10code.com>
+
+	* scdaemon.c (handle_connections): Make sure that the signals we
+	are handling are not blocked.Block signals while creating new
+	threads.
+	(handle_connections): Include the file descriptor into the name of
+	the thread.
+
+2005-06-02  Werner Koch  <wk@g10code.com>
+
+	* app.c (app_dump_state, dump_mutex_state): New.
+	* scdaemon.c (handle_signal): Print it on SIGUSR1.
+
+	* app-openpgp.c (do_writekey): Typo fix.
+
+	* command.c (open_card): Check for locked state even if an
+	application context is available.
+
+	* app-common.h: Add REF_COUNT field.
+	* app.c (release_application, select_application): Implement
+	reference counting to share the context beween connections.
+
+	* app.c (lock_reader, unlock_reader): Take SLOT instead of APP as
+	argument.  Changed all callers.
+	(select_application): Unlock the reader on error.  This should fix
+	the hangs I noticed last week.
+
+	* scdaemon.h: Removed card_ctx_t cruft.
+
 2005-06-01  Werner Koch  <wk@g10code.com>
 
 	* scdaemon.c: Include mkdtemp.h.
diff --git a/scd/app-common.h b/scd/app-common.h
index 812736ece..94087f221 100644
--- a/scd/app-common.h
+++ b/scd/app-common.h
@@ -39,6 +39,11 @@ struct app_ctx_s {
                        function pointers may be used.  Note that for
                        unsupported operations the particular
                        function pointer is set to NULL */
+
+  int ref_count;    /* Number of connections currently using this
+                       application context.  fixme: We might want to
+                       merg this witghn INITIALIZED above. */
+
   int slot;         /* Used reader. */
 
   /* If this is used by GnuPG 1.4 we need to know the assuan context
@@ -123,6 +128,7 @@ size_t app_help_read_length_of_cert (int slot, int fid, size_t *r_certoff);
 
 
 /*-- app.c --*/
+void app_dump_state (void);
 gpg_error_t select_application (ctrl_t ctrl, int slot, const char *name,
                                 app_t *r_app);
 void release_application (app_t app);
diff --git a/scd/app-openpgp.c b/scd/app-openpgp.c
index 14483869b..1ff096138 100644
--- a/scd/app-openpgp.c
+++ b/scd/app-openpgp.c
@@ -1745,7 +1745,7 @@ do_writekey (app_t app, ctrl_t ctrl,
   nbits = rsa_e? count_bits (rsa_e, rsa_e_len) : 0;
   if (nbits < 2 || nbits > 32)
     {
-      log_error (_("RSA public exponent missing or largerr than %d bits\n"),
+      log_error (_("RSA public exponent missing or larger than %d bits\n"),
                  32);
       err = gpg_error (GPG_ERR_BAD_SECKEY);
       goto leave;
diff --git a/scd/app.c b/scd/app.c
index f2c427f5b..2c8c915d7 100644
--- a/scd/app.c
+++ b/scd/app.c
@@ -39,25 +39,24 @@ static struct
 {
   int initialized;
   pth_mutex_t lock;
+  app_t app;        /* Application context in use or NULL. */
 } lock_table[10];
 
 
-/* Lock the reader associated with the APP context.  This function
-   shall be used right before calling any of the actual application
-   functions to serialize access to the reader.  We do this always
-   even if the reader is not actually used.  This allows an actual
-   application to assume that it never shares a reader (while
-   performing one command).  Returns 0 on success; only then the
-   unlock_reader function must be called after returning from the
-   handler. */
+/* Lock the reader SLOT.  This function shall be used right before
+   calling any of the actual application functions to serialize access
+   to the reader.  We do this always even if the reader is not
+   actually used.  This allows an actual connection to assume that it
+   never shares a reader (while performing one command).  Returns 0 on
+   success; only then the unlock_reader function must be called after
+   returning from the handler. */
 static gpg_error_t 
-lock_reader (app_t app)
+lock_reader (int slot)
 {
   gpg_error_t err;
-  int slot = app->slot;
 
   if (slot < 0 || slot >= DIM (lock_table))
-    return gpg_error (app->slot<0? GPG_ERR_INV_VALUE : GPG_ERR_RESOURCE_LIMIT);
+    return gpg_error (slot<0? GPG_ERR_INV_VALUE : GPG_ERR_RESOURCE_LIMIT);
 
   if (!lock_table[slot].initialized)
     {
@@ -68,6 +67,7 @@ lock_reader (app_t app)
           return err;
         }
       lock_table[slot].initialized = 1;
+      lock_table[slot].app = NULL;
     }
   
   if (!pth_mutex_acquire (&lock_table[slot].lock, 0, NULL))
@@ -83,10 +83,8 @@ lock_reader (app_t app)
 
 /* Release a lock on the reader.  See lock_reader(). */
 static void
-unlock_reader (app_t app)
+unlock_reader (int slot)
 {
-  int slot = app->slot;
-
   if (slot < 0 || slot >= DIM (lock_table)
       || !lock_table[slot].initialized)
     log_bug ("unlock_reader called for invalid slot %d\n", slot);
@@ -96,6 +94,39 @@ unlock_reader (app_t app)
                slot, strerror (errno));
 
 }
+
+
+static void
+dump_mutex_state (pth_mutex_t *m)
+{
+  if (!(m->mx_state & PTH_MUTEX_INITIALIZED))
+    log_printf ("not_initialized");
+  else if (!(m->mx_state & PTH_MUTEX_LOCKED))
+    log_printf ("not_locked");
+  else
+    log_printf ("locked tid=0x%lx count=%lu", (long)m->mx_owner, m->mx_count);
+}
+
+
+/* This function may be called to print information pertaining to the
+   current state of this module to the log. */
+void
+app_dump_state (void)
+{
+  int slot;
+
+  for (slot=0; slot < DIM (lock_table); slot++)
+    if (lock_table[slot].initialized)
+      {
+        log_info ("app_dump_state: slot=%d lock=", slot);
+        dump_mutex_state (&lock_table[slot].lock);
+        if (lock_table[slot].app)
+          log_printf (" app=%p type=`%s'",
+                      lock_table[slot].app, lock_table[slot].app->apptype);
+        log_printf ("\n");
+      }
+}
+
  
 /* Check wether the application NAME is allowed.  This does not mean
    we have support for it though.  */
@@ -120,23 +151,48 @@ gpg_error_t
 select_application (ctrl_t ctrl, int slot, const char *name, app_t *r_app)
 {
   gpg_error_t err;
-  app_t app;
+  app_t app = NULL;
   unsigned char *result = NULL;
   size_t resultlen;
 
   *r_app = NULL;
+
+  err = lock_reader (slot);
+  if (err)
+    return err;
+
+  /* First check whether we already have an application to share. */
+  app = lock_table[slot].initialized ? lock_table[slot].app : NULL;
+  if (app && name)
+    if (!app->apptype || ascii_strcasecmp (app->apptype, name))
+      {
+        unlock_reader (slot);
+        if (app->apptype)
+          log_info ("application `%s' in use by reader %d - can't switch\n",
+                    app->apptype, slot);
+        return gpg_error (GPG_ERR_CONFLICT);
+      }
+
+  if (app)
+    {
+      if (app->slot != slot)
+        log_bug ("slot mismatch %d/%d\n", app->slot, slot);
+      app->ref_count++;
+      *r_app = app;
+      unlock_reader (slot);
+      return 0; /* Okay: We share that one. */
+    }
+
   app = xtrycalloc (1, sizeof *app);
   if (!app)
     {
       err = gpg_error_from_errno (errno);
       log_info ("error allocating context: %s\n", gpg_strerror (err));
+      unlock_reader (slot);
       return err;
     }
   app->slot = slot;
 
-  err = lock_reader (app);
-  if (err)
-    return err;
 
   /* Fixme: We should now first check whether a card is at all
      present. */
@@ -162,7 +218,7 @@ select_application (ctrl_t ctrl, int slot, const char *name, app_t *r_app)
              have some test cards with such an invalid encoding and
              therefore I use this ugly workaround to return something
              I can further experiment with. */
-          log_debug ("enabling BMI testcard workaround\n");
+          log_info ("enabling BMI testcard workaround\n");
           n--;
         }
 
@@ -212,22 +268,41 @@ select_application (ctrl_t ctrl, int slot, const char *name, app_t *r_app)
         log_info ("no supported card application found: %s\n",
                   gpg_strerror (err));
       xfree (app);
+      unlock_reader (slot);
       return err;
     }
 
   app->initialized = 1;
-  unlock_reader (app);
+  app->ref_count = 1;
+  lock_table[slot].app = app;
   *r_app = app;
+  unlock_reader (slot);
   return 0;
 }
 
 
+/* Free the resources associated with the application APP.  APP is
+   allowed to be NULL in which case this is a no-op.  Note that we are
+   using reference counting to track the users of the application. */
 void
 release_application (app_t app)
 {
+  int slot;
+
   if (!app)
     return;
 
+  if (app->ref_count < 1)
+    log_bug ("trying to release an already released context\n");
+  if (--app->ref_count)
+    return;
+
+  /* Clear the reference to the application from the lock table. */
+  for (slot = 0;  slot < DIM (lock_table); slot++)
+    if (lock_table[slot].initialized && lock_table[slot].app == app)
+      lock_table[slot].app = NULL;
+
+  /* Deallocate. */
   if (app->fnc.deinit)
     {
       app->fnc.deinit (app);
@@ -320,11 +395,11 @@ app_write_learn_status (app_t app, CTRL ctrl)
   if (app->apptype)
     send_status_info (ctrl, "APPTYPE",
                       app->apptype, strlen (app->apptype), NULL, 0);
-  err = lock_reader (app);
+  err = lock_reader (app->slot);
   if (err)
     return err;
   err = app->fnc.learn_status (app, ctrl);
-  unlock_reader (app);
+  unlock_reader (app->slot);
   return err;
 }
 
@@ -345,11 +420,11 @@ app_readcert (app_t app, const char *certid,
     return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED);
   if (!app->fnc.readcert)
     return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION);
-  err = lock_reader (app);
+  err = lock_reader (app->slot);
   if (err)
     return err;
   err = app->fnc.readcert (app, certid, cert, certlen);
-  unlock_reader (app);
+  unlock_reader (app->slot);
   return err;
 }
 
@@ -377,11 +452,11 @@ app_readkey (app_t app, const char *keyid, unsigned char **pk, size_t *pklen)
     return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED);
   if (!app->fnc.readkey)
     return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION);
-  err = lock_reader (app);
+  err = lock_reader (app->slot);
   if (err)
     return err;
   err= app->fnc.readkey (app, keyid, pk, pklen);
-  unlock_reader (app);
+  unlock_reader (app->slot);
   return err;
 }
 
@@ -419,11 +494,11 @@ app_getattr (app_t app, CTRL ctrl, const char *name)
 
   if (!app->fnc.getattr)
     return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION);
-  err = lock_reader (app);
+  err = lock_reader (app->slot);
   if (err)
     return err;
   err =  app->fnc.getattr (app, ctrl, name);
-  unlock_reader (app);
+  unlock_reader (app->slot);
   return err;
 }
 
@@ -442,11 +517,11 @@ app_setattr (app_t app, const char *name,
     return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED);
   if (!app->fnc.setattr)
     return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION);
-  err = lock_reader (app);
+  err = lock_reader (app->slot);
   if (err)
     return err;
   err = app->fnc.setattr (app, name, pincb, pincb_arg, value, valuelen);
-  unlock_reader (app);
+  unlock_reader (app->slot);
   return err;
 }
 
@@ -468,14 +543,14 @@ app_sign (app_t app, const char *keyidstr, int hashalgo,
     return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED);
   if (!app->fnc.sign)
     return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION);
-  err = lock_reader (app);
+  err = lock_reader (app->slot);
   if (err)
     return err;
   err = app->fnc.sign (app, keyidstr, hashalgo,
                        pincb, pincb_arg,
                        indata, indatalen,
                        outdata, outdatalen);
-  unlock_reader (app);
+  unlock_reader (app->slot);
   if (opt.verbose)
     log_info ("operation sign result: %s\n", gpg_strerror (err));
   return err;
@@ -500,14 +575,14 @@ app_auth (app_t app, const char *keyidstr,
     return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED);
   if (!app->fnc.auth)
     return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION);
-  err = lock_reader (app);
+  err = lock_reader (app->slot);
   if (err)
     return err;
   err = app->fnc.auth (app, keyidstr,
                        pincb, pincb_arg,
                        indata, indatalen,
                        outdata, outdatalen);
-  unlock_reader (app);
+  unlock_reader (app->slot);
   if (opt.verbose)
     log_info ("operation auth result: %s\n", gpg_strerror (err));
   return err;
@@ -532,14 +607,14 @@ app_decipher (app_t app, const char *keyidstr,
     return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED);
   if (!app->fnc.decipher)
     return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION);
-  err = lock_reader (app);
+  err = lock_reader (app->slot);
   if (err)
     return err;
   err = app->fnc.decipher (app, keyidstr,
                            pincb, pincb_arg,
                            indata, indatalen,
                            outdata, outdatalen);
-  unlock_reader (app);
+  unlock_reader (app->slot);
   if (opt.verbose)
     log_info ("operation decipher result: %s\n", gpg_strerror (err));
   return err;
@@ -562,12 +637,12 @@ app_writekey (app_t app, ctrl_t ctrl,
     return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED);
   if (!app->fnc.writekey)
     return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION);
-  err = lock_reader (app);
+  err = lock_reader (app->slot);
   if (err)
     return err;
   err = app->fnc.writekey (app, ctrl, keyidstr, flags,
                            pincb, pincb_arg, keydata, keydatalen);
-  unlock_reader (app);
+  unlock_reader (app->slot);
   if (opt.verbose)
     log_info ("operation writekey result: %s\n", gpg_strerror (err));
   return err;
@@ -589,11 +664,11 @@ app_genkey (app_t app, CTRL ctrl, const char *keynostr, unsigned int flags,
     return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED);
   if (!app->fnc.genkey)
     return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION);
-  err = lock_reader (app);
+  err = lock_reader (app->slot);
   if (err)
     return err;
   err = app->fnc.genkey (app, ctrl, keynostr, flags, pincb, pincb_arg);
-  unlock_reader (app);
+  unlock_reader (app->slot);
   if (opt.verbose)
     log_info ("operation genkey result: %s\n", gpg_strerror (err));
   return err;
@@ -612,11 +687,11 @@ app_get_challenge (app_t app, size_t nbytes, unsigned char *buffer)
     return gpg_error (GPG_ERR_INV_VALUE);
   if (!app->initialized)
     return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED);
-  err = lock_reader (app);
+  err = lock_reader (app->slot);
   if (err)
     return err;
   err = iso7816_get_challenge (app->slot, nbytes, buffer);
-  unlock_reader (app);
+  unlock_reader (app->slot);
   return err;
 }
 
@@ -636,12 +711,12 @@ app_change_pin (app_t app, CTRL ctrl, const char *chvnostr, int reset_mode,
     return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED);
   if (!app->fnc.change_pin)
     return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION);
-  err = lock_reader (app);
+  err = lock_reader (app->slot);
   if (err)
     return err;
   err = app->fnc.change_pin (app, ctrl, chvnostr, reset_mode,
                              pincb, pincb_arg);
-  unlock_reader (app);
+  unlock_reader (app->slot);
   if (opt.verbose)
     log_info ("operation change_pin result: %s\n", gpg_strerror (err));
   return err;
@@ -664,11 +739,11 @@ app_check_pin (app_t app, const char *keyidstr,
     return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED);
   if (!app->fnc.check_pin)
     return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION);
-  err = lock_reader (app);
+  err = lock_reader (app->slot);
   if (err)
     return err;
   err = app->fnc.check_pin (app, keyidstr, pincb, pincb_arg);
-  unlock_reader (app);
+  unlock_reader (app->slot);
   if (opt.verbose)
     log_info ("operation check_pin result: %s\n", gpg_strerror (err));
   return err;
diff --git a/scd/command.c b/scd/command.c
index 738b1f003..287f8c921 100644
--- a/scd/command.c
+++ b/scd/command.c
@@ -253,12 +253,12 @@ open_card (ctrl_t ctrl, const char *apptype)
   if (ctrl->server_local->card_removed)
     return map_to_assuan_status (gpg_error (GPG_ERR_CARD_REMOVED));
 
-  if (ctrl->app_ctx)
-    return 0; /* Already initialized for one specific application. */
-
   if ( IS_LOCKED (ctrl) )
     return gpg_error (GPG_ERR_LOCKED);
 
+  if (ctrl->app_ctx)
+    return 0; /* Already initialized for one specific application. */
+
   if (ctrl->reader_slot != -1)
     slot = ctrl->reader_slot;
   else
diff --git a/scd/scdaemon.c b/scd/scdaemon.c
index 341719b1e..5b5e09176 100644
--- a/scd/scdaemon.c
+++ b/scd/scdaemon.c
@@ -809,6 +809,7 @@ handle_signal (int signo)
     case SIGUSR1:
       log_info ("SIGUSR1 received - printing internal information:\n");
       pth_ctrl (PTH_CTRL_DUMPSTATE, log_get_stream ());
+      app_dump_state ();
       break;
 
     case SIGUSR2:
@@ -1013,7 +1014,6 @@ handle_connections (int listen_fd)
   tattr = pth_attr_new();
   pth_attr_set (tattr, PTH_ATTR_JOINABLE, 0);
   pth_attr_set (tattr, PTH_ATTR_STACK_SIZE, 512*1024);
-  pth_attr_set (tattr, PTH_ATTR_NAME, "scd-connections");
 
 #ifndef HAVE_W32_SYSTEM /* fixme */
   sigemptyset (&sigs );
@@ -1022,6 +1022,7 @@ handle_connections (int listen_fd)
   sigaddset (&sigs, SIGUSR2);
   sigaddset (&sigs, SIGINT);
   sigaddset (&sigs, SIGTERM);
+  pth_sigmask (SIG_UNBLOCK, &sigs, NULL);
   ev = pth_event (PTH_EVENT_SIGS, &sigs, &signo);
 #else
   ev = NULL;
@@ -1034,6 +1035,8 @@ handle_connections (int listen_fd)
 
   for (;;)
     {
+      sigset_t oldsigs;
+      
       if (shutdown_pending)
         {
           if (pth_ctrl (PTH_CTRL_GETTHREADS) == 1)
@@ -1093,6 +1096,11 @@ handle_connections (int listen_fd)
           handle_tick ();
         }
 
+      /* We now might create new threads and because we don't want any
+         signals - we are handling here - to be delivered to a new
+         thread. Thus we need to block those signals. */
+      pth_sigmask (SIG_BLOCK, &sigs, &oldsigs);
+
       if (listen_fd != -1 && FD_ISSET (listen_fd, &read_fdset))
 	{
           plen = sizeof paddr;
@@ -1101,15 +1109,26 @@ handle_connections (int listen_fd)
 	    {
 	      log_error ("accept failed: %s\n", strerror (errno));
 	    }
-          else if (!pth_spawn (tattr, start_connection_thread, (void*)fd))
-	    {
-	      log_error ("error spawning connection handler: %s\n",
-			 strerror (errno) );
-	      close (fd);
-	    }
+          else
+            {
+              char threadname[50];
+              snprintf (threadname, sizeof threadname-1, "conn fd=%d", fd);
+              threadname[sizeof threadname -1] = 0;
+              pth_attr_set (tattr, PTH_ATTR_NAME, threadname);
+              
+              if (!pth_spawn (tattr, start_connection_thread, (void*)fd))
+                {
+                  log_error ("error spawning connection handler: %s\n",
+                             strerror (errno) );
+                  close (fd);
+                }
+            }
           fd = -1;
 	}
 
+      /* Restore the signal mask. */
+      pth_sigmask (SIG_SETMASK, &oldsigs, NULL);
+
     }
 
   pth_event_free (ev, PTH_FREE_ALL);
diff --git a/scd/scdaemon.h b/scd/scdaemon.h
index eaa9abd35..54566b6ad 100644
--- a/scd/scdaemon.h
+++ b/scd/scdaemon.h
@@ -77,19 +77,28 @@ struct {
 #define DBG_CARD_IO (opt.debug & DBG_CARD_IO_VALUE)
 
 struct server_local_s;
-struct card_ctx_s;
 struct app_ctx_s;
 
-struct server_control_s {
+struct server_control_s 
+{
+  /* Local data of the server; used only in command.c. */
   struct server_local_s *server_local;
-  int reader_slot;  /* Slot of the open reader or -1 if not open. */
-  struct card_ctx_s *card_ctx;
+
+  /* Slot of the open reader or -1 if not open. */
+  int reader_slot; 
+
+  /* The application context used with this connection or NULL if none
+     associated.  Note that this is shared with the other connections:
+     All connections accessing the same reader are using the same
+     application context. */
   struct app_ctx_s *app_ctx;
-  struct {
+
+  /* Helper to store the value we are going to sign */
+  struct 
+  {
     unsigned char *value;  
     int valuelen;
-  } in_data;  /* helper to store the value we are going to sign */
-
+  } in_data;  
 };
 
 typedef struct server_control_s *CTRL;