more KS cleanups, including one race fix

src/exchange/taler-exchange-httpd_deposit.c

@@ -288,10 +288,10 @@ verify_and_execute_deposit (struct MHD_Connection *connection,
 
   /* check denomination */
   {
-    struct TEH_KS_StateHandle *mks;
+    struct TEH_KS_StateHandle *key_state;
 
-    mks = TEH_KS_acquire (GNUNET_TIME_absolute_get ());
+    key_state = TEH_KS_acquire (GNUNET_TIME_absolute_get ());
-    if (NULL == mks)
+    if (NULL == key_state)
     {
       TALER_LOG_ERROR ("Lacking keys to operate\n");
       return TALER_MHD_reply_with_error (connection,
@@ -299,14 +299,14 @@ verify_and_execute_deposit (struct MHD_Connection *connection,
                                          TALER_EC_EXCHANGE_BAD_CONFIGURATION,
                                          "no keys");
     }
-    dki = TEH_KS_denomination_key_lookup_by_hash (mks,
+    dki = TEH_KS_denomination_key_lookup_by_hash (key_state,
                                                   &deposit->coin.denom_pub_hash,
                                                   TEH_KS_DKU_DEPOSIT,
                                                   &ec,
                                                   &hc);
     if (NULL == dki)
     {
-      TEH_KS_release (mks);
+      TEH_KS_release (key_state);
       return TALER_MHD_reply_with_error (connection,
                                          hc,
                                          ec,
@@ -314,7 +314,7 @@ verify_and_execute_deposit (struct MHD_Connection *connection,
     }
     TALER_amount_ntoh (&dc.value,
                        &dki->issue.properties.value);
-    TEH_KS_release (mks);
+    TEH_KS_release (key_state);
   }
   /* execute transaction */
   dc.deposit = deposit;

src/exchange/taler-exchange-httpd_keystate.c

@@ -2273,13 +2273,19 @@ read_again:
 void
 TEH_KS_free ()
 {
+  GNUNET_assert (0 == pthread_mutex_lock (&internal_key_state_mutex));
   if (NULL != internal_key_state)
   {
     struct TEH_KS_StateHandle *ks = internal_key_state;
 
     internal_key_state = NULL;
+    GNUNET_assert (0 == pthread_mutex_unlock (&internal_key_state_mutex));
     TEH_KS_release (ks);
   }
+  else
+  {
+    GNUNET_assert (0 == pthread_mutex_unlock (&internal_key_state_mutex));
+  }
 }
 
 

src/exchange/taler-exchange-httpd_refresh_reveal.c

@@ -518,102 +518,34 @@ refresh_reveal_persist (void *cls,
 
 
 /**
- * Handle a "/refresh/reveal" request.   Parses the given JSON
+ * Resolve denomination hashes using the @a key_state
- * transfer private keys and if successful, passes everything to
- * #TEH_DB_execute_refresh_reveal() which will verify that the
- * revealed information is valid then returns the signed refreshed
- * coins.
  *
+ * @param key_state the key state
  * @param connection the MHD connection to handle
  * @param rctx context for the operation, partially built at this time
  * @param transfer_pub transfer public key
- * @param tp_json private transfer keys in JSON format
  * @param link_sigs_json link signatures in JSON format
  * @param new_denoms_h_json requests for fresh coins to be created
  * @param coin_evs envelopes of gamma-selected coins to be signed
  * @return MHD result code
  */
 static int
-handle_refresh_reveal_json (struct MHD_Connection *connection,
+resolve_refresh_reveal_denominations (struct TEH_KS_StateHandle *key_state,
+                                      struct MHD_Connection *connection,
                                       struct RevealContext *rctx,
-                            const json_t *tp_json,
                                       const json_t *link_sigs_json,
                                       const json_t *new_denoms_h_json,
                                       const json_t *coin_evs)
 {
   unsigned int num_fresh_coins = json_array_size (new_denoms_h_json);
-  unsigned int num_tprivs = json_array_size (tp_json);
-  struct TEH_KS_StateHandle *key_state;
-  struct TALER_EXCHANGEDB_RefreshMelt refresh_melt;
-
-  GNUNET_assert (num_tprivs == TALER_CNC_KAPPA - 1);
-  if ( (num_fresh_coins >= MAX_FRESH_COINS) ||
-       (0 == num_fresh_coins) )
-  {
-    GNUNET_break_op (0);
-    return TALER_MHD_reply_with_error (connection,
-                                       MHD_HTTP_BAD_REQUEST,
-                                       TALER_EC_REFRESH_REVEAL_NEW_DENOMS_ARRAY_SIZE_EXCESSIVE,
-                                       "new_denoms_h");
-
-  }
-  if (json_array_size (new_denoms_h_json) !=
-      json_array_size (coin_evs))
-  {
-    GNUNET_break_op (0);
-    return TALER_MHD_reply_with_error (connection,
-                                       MHD_HTTP_BAD_REQUEST,
-                                       TALER_EC_REFRESH_REVEAL_NEW_DENOMS_ARRAY_SIZE_MISSMATCH,
-                                       "new_denoms/coin_evs");
-  }
-  if (json_array_size (new_denoms_h_json) !=
-      json_array_size (link_sigs_json))
-  {
-    GNUNET_break_op (0);
-    return TALER_MHD_reply_with_error (connection,
-                                       MHD_HTTP_BAD_REQUEST,
-                                       TALER_EC_REFRESH_REVEAL_NEW_DENOMS_ARRAY_SIZE_MISSMATCH,
-                                       "new_denoms/link_sigs");
-  }
-
-  /* Parse transfer private keys array */
-  for (unsigned int i = 0; i<num_tprivs; i++)
-  {
-    struct GNUNET_JSON_Specification trans_spec[] = {
-      GNUNET_JSON_spec_fixed_auto (NULL, &rctx->transfer_privs[i]),
-      GNUNET_JSON_spec_end ()
-    };
-    int res;
-
-    res = TALER_MHD_parse_json_array (connection,
-                                      tp_json,
-                                      trans_spec,
-                                      i,
-                                      -1);
-    if (GNUNET_OK != res)
-      return (GNUNET_NO == res) ? MHD_YES : MHD_NO;
-  }
-
-  /* Resolve denomination hashes */
-  {
   const struct
   TALER_EXCHANGEDB_DenominationKeyIssueInformation *dkis[num_fresh_coins];
   struct GNUNET_HashCode dki_h[num_fresh_coins];
   struct TALER_RefreshCoinData rcds[num_fresh_coins];
   struct TALER_CoinSpendSignatureP link_sigs[num_fresh_coins];
+  struct TALER_EXCHANGEDB_RefreshMelt refresh_melt;
   int res;
 
-    /* Resolve denomination hashes */
-    key_state = TEH_KS_acquire (GNUNET_TIME_absolute_get ());
-    if (NULL == key_state)
-    {
-      TALER_LOG_ERROR ("Lacking keys to operate\n");
-      return TALER_MHD_reply_with_error (connection,
-                                         MHD_HTTP_INTERNAL_SERVER_ERROR,
-                                         TALER_EC_REFRESH_REVEAL_KEYS_MISSING,
-                                         "exchange lacks keys");
-    }
-
   /* Parse denomination key hashes */
   for (unsigned int i = 0; i<num_fresh_coins; i++)
   {
@@ -632,7 +564,6 @@ handle_refresh_reveal_json (struct MHD_Connection *connection,
                                       -1);
     if (GNUNET_OK != res)
     {
-        TEH_KS_release (key_state);
       return (GNUNET_NO == res) ? MHD_YES : MHD_NO;
     }
     dkis[i] = TEH_KS_denomination_key_lookup_by_hash (key_state,
@@ -642,7 +573,6 @@ handle_refresh_reveal_json (struct MHD_Connection *connection,
                                                       &hc);
     if (NULL == dkis[i])
     {
-        TEH_KS_release (key_state);
       return TALER_MHD_reply_with_error (connection,
                                          hc,
                                          ec,
@@ -671,7 +601,6 @@ handle_refresh_reveal_json (struct MHD_Connection *connection,
     {
       for (unsigned int j = 0; j<i; j++)
         GNUNET_free_non_null (rcds[j].coin_ev);
-        TEH_KS_release (key_state);
       return (GNUNET_NO == res) ? MHD_YES : MHD_NO;
     }
     rcd->dk = &dkis[i]->denom_pub;
@@ -839,7 +768,6 @@ handle_refresh_reveal_json (struct MHD_Connection *connection,
       break;
     }
   }   /* end for (retries...) */
-    GNUNET_break (MHD_NO != res);
 
 cleanup:
   GNUNET_break (MHD_NO != res);
@@ -850,12 +778,111 @@ cleanup:
       if (NULL != rctx->ev_sigs[i].rsa_signature)
         GNUNET_CRYPTO_rsa_signature_free (rctx->ev_sigs[i].rsa_signature);
     GNUNET_free (rctx->ev_sigs);
+    rctx->ev_sigs = NULL; /* just to be safe... */
   }
   for (unsigned int i = 0; i<num_fresh_coins; i++)
     GNUNET_free_non_null (rcds[i].coin_ev);
-    TEH_KS_release (key_state);
   return res;
 }
+
+
+/**
+ * Handle a "/refresh/reveal" request.   Parses the given JSON
+ * transfer private keys and if successful, passes everything to
+ * #TEH_DB_execute_refresh_reveal() which will verify that the
+ * revealed information is valid then returns the signed refreshed
+ * coins.
+ *
+ * @param connection the MHD connection to handle
+ * @param rctx context for the operation, partially built at this time
+ * @param transfer_pub transfer public key
+ * @param tp_json private transfer keys in JSON format
+ * @param link_sigs_json link signatures in JSON format
+ * @param new_denoms_h_json requests for fresh coins to be created
+ * @param coin_evs envelopes of gamma-selected coins to be signed
+ * @return MHD result code
+ */
+static int
+handle_refresh_reveal_json (struct MHD_Connection *connection,
+                            struct RevealContext *rctx,
+                            const json_t *tp_json,
+                            const json_t *link_sigs_json,
+                            const json_t *new_denoms_h_json,
+                            const json_t *coin_evs)
+{
+  unsigned int num_fresh_coins = json_array_size (new_denoms_h_json);
+  unsigned int num_tprivs = json_array_size (tp_json);
+
+  GNUNET_assert (num_tprivs == TALER_CNC_KAPPA - 1);
+  if ( (num_fresh_coins >= MAX_FRESH_COINS) ||
+       (0 == num_fresh_coins) )
+  {
+    GNUNET_break_op (0);
+    return TALER_MHD_reply_with_error (connection,
+                                       MHD_HTTP_BAD_REQUEST,
+                                       TALER_EC_REFRESH_REVEAL_NEW_DENOMS_ARRAY_SIZE_EXCESSIVE,
+                                       "new_denoms_h");
+
+  }
+  if (json_array_size (new_denoms_h_json) !=
+      json_array_size (coin_evs))
+  {
+    GNUNET_break_op (0);
+    return TALER_MHD_reply_with_error (connection,
+                                       MHD_HTTP_BAD_REQUEST,
+                                       TALER_EC_REFRESH_REVEAL_NEW_DENOMS_ARRAY_SIZE_MISSMATCH,
+                                       "new_denoms/coin_evs");
+  }
+  if (json_array_size (new_denoms_h_json) !=
+      json_array_size (link_sigs_json))
+  {
+    GNUNET_break_op (0);
+    return TALER_MHD_reply_with_error (connection,
+                                       MHD_HTTP_BAD_REQUEST,
+                                       TALER_EC_REFRESH_REVEAL_NEW_DENOMS_ARRAY_SIZE_MISSMATCH,
+                                       "new_denoms/link_sigs");
+  }
+
+  /* Parse transfer private keys array */
+  for (unsigned int i = 0; i<num_tprivs; i++)
+  {
+    struct GNUNET_JSON_Specification trans_spec[] = {
+      GNUNET_JSON_spec_fixed_auto (NULL, &rctx->transfer_privs[i]),
+      GNUNET_JSON_spec_end ()
+    };
+    int res;
+
+    res = TALER_MHD_parse_json_array (connection,
+                                      tp_json,
+                                      trans_spec,
+                                      i,
+                                      -1);
+    if (GNUNET_OK != res)
+      return (GNUNET_NO == res) ? MHD_YES : MHD_NO;
+  }
+
+  {
+    struct TEH_KS_StateHandle *key_state;
+    int ret;
+
+    key_state = TEH_KS_acquire (GNUNET_TIME_absolute_get ());
+    if (NULL == key_state)
+    {
+      TALER_LOG_ERROR ("Lacking keys to operate\n");
+      return TALER_MHD_reply_with_error (connection,
+                                         MHD_HTTP_INTERNAL_SERVER_ERROR,
+                                         TALER_EC_REFRESH_REVEAL_KEYS_MISSING,
+                                         "exchange lacks keys");
+    }
+    ret = resolve_refresh_reveal_denominations (key_state,
+                                                connection,
+                                                rctx,
+                                                link_sigs_json,
+                                                new_denoms_h_json,
+                                                coin_evs);
+    TEH_KS_release (key_state);
+    return ret;
+  }
 }
 
 

src/exchange/taler-exchange-httpd_refund.c

@@ -486,10 +486,10 @@ verify_and_execute_refund (struct MHD_Connection *connection,
   }
 
   {
-    struct TEH_KS_StateHandle *mks;
+    struct TEH_KS_StateHandle *key_state;
 
-    mks = TEH_KS_acquire (GNUNET_TIME_absolute_get ());
+    key_state = TEH_KS_acquire (GNUNET_TIME_absolute_get ());
-    if (NULL == mks)
+    if (NULL == key_state)
     {
       TALER_LOG_ERROR ("Lacking keys to operate\n");
       return TALER_MHD_reply_with_error (connection,
@@ -503,7 +503,7 @@ verify_and_execute_refund (struct MHD_Connection *connection,
       unsigned int hc;
       enum TALER_ErrorCode ec;
 
-      dki = TEH_KS_denomination_key_lookup_by_hash (mks,
+      dki = TEH_KS_denomination_key_lookup_by_hash (key_state,
                                                     &denom_hash,
                                                     TEH_KS_DKU_DEPOSIT,
                                                     &ec,
@@ -513,7 +513,7 @@ verify_and_execute_refund (struct MHD_Connection *connection,
         /* DKI not found, but we do have a coin with this DK in our database;
            not good... */
         GNUNET_break (0);
-        TEH_KS_release (mks);
+        TEH_KS_release (key_state);
         return TALER_MHD_reply_with_error (connection,
                                            hc,
                                            ec,
@@ -522,7 +522,7 @@ verify_and_execute_refund (struct MHD_Connection *connection,
       TALER_amount_ntoh (&rc.expect_fee,
                          &dki->issue.properties.fee_refund);
     }
-    TEH_KS_release (mks);
+    TEH_KS_release (key_state);
   }
 
   /* Finally run the actual transaction logic */