clean up the wawaka kvstore table more thoroughly

move kvstore clean up into its own function. this replaces
the exception with a more thorough cleanup to ensure that the
memory is released.

make a few variables in the state extensions constant that should
have been constant from the start

Signed-off-by: Mic Bowman <mic.bowman@intel.com>

Author: cmickeyb

common/interpreter/wawaka_wasm/WasmStateExtensions.cpp

@@ -81,14 +81,14 @@ extern "C" bool key_value_set_wrapper(
         if (state == NULL)
             return false;
 
-        if (key_buffer == NULL)
+        if (key_buffer == NULL || key_buffer_length == 0)
             return false;
 
         if (val_buffer == NULL)
             return false;
 
-        ByteArray ba_key(key_buffer, key_buffer + key_buffer_length);
-        ByteArray ba_val(val_buffer, val_buffer + val_buffer_length);
+        const ByteArray ba_key(key_buffer, key_buffer + key_buffer_length);
+        const ByteArray ba_val(val_buffer, val_buffer + val_buffer_length);
 
         state->UnprivilegedPut(ba_key, ba_val);
         return true;
@@ -120,10 +120,10 @@ extern "C" bool key_value_get_wrapper(
         if (state == NULL)
             return false;
 
-        if (key_buffer == NULL)
+        if (key_buffer == NULL || key_buffer_length == 0)
             return false;
 
-        ByteArray ba_key(key_buffer, key_buffer + key_buffer_length);
+        const ByteArray ba_key(key_buffer, key_buffer + key_buffer_length);
         ByteArray ba_val = state->UnprivilegedGet(ba_key);
 
         if (ba_val.size() == 0)
@@ -162,10 +162,10 @@ extern "C" bool privileged_key_value_get_wrapper(
         if (state == NULL)
             return false;
 
-        if (key_buffer == NULL)
+        if (key_buffer == NULL || key_buffer_length == 0)
             return false;
 
-        ByteArray ba_key(key_buffer, key_buffer + key_buffer_length);
+        const ByteArray ba_key(key_buffer, key_buffer + key_buffer_length);
         ByteArray ba_val = state->PrivilegedGet(ba_key);
 
         if (ba_val.size() == 0)
@@ -206,7 +206,7 @@ extern "C" int32 key_value_create_wrapper(
             return false;
         }
 
-        ByteArray ba_encryption_key(aes_key_buffer, aes_key_buffer + aes_key_buffer_length);
+        const ByteArray ba_encryption_key(aes_key_buffer, aes_key_buffer + aes_key_buffer_length);
 
         // find an empty slot we can use for the kv store
         pstate::Basic_KV_Plus** kv_store_pool = (pstate::Basic_KV_Plus**)wasm_runtime_get_custom_data(module_inst);
@@ -258,15 +258,15 @@ extern "C" int32 key_value_open_wrapper(
         if (id_hash_buffer == NULL || id_hash_buffer_length <= 0)
             return false;
 
-        ByteArray ba_id_hash(id_hash_buffer, id_hash_buffer + id_hash_buffer_length);
+        const ByteArray ba_id_hash(id_hash_buffer, id_hash_buffer + id_hash_buffer_length);
 
         if (aes_key_buffer == NULL || aes_key_buffer_length != pdo::crypto::constants::SYM_KEY_LEN)
         {
             SAFE_LOG(PDO_LOG_ERROR, "invalid encryption key; %d", aes_key_buffer_length);
             return false;
         }
 
-        ByteArray ba_encryption_key(aes_key_buffer, aes_key_buffer + aes_key_buffer_length);
+        const ByteArray ba_encryption_key(aes_key_buffer, aes_key_buffer + aes_key_buffer_length);
 
         // find an empty slot we can use for the kv store
         pstate::Basic_KV_Plus** kv_store_pool = (pstate::Basic_KV_Plus**)wasm_runtime_get_custom_data(module_inst);
@@ -325,6 +325,7 @@ extern "C" bool key_value_finalize_wrapper(
         // Call finalize and cross your fingers
         ByteArray ba_val;
 
+        SAFE_LOG(PDO_LOG_INFO, "finalizing key/value store");
         state->Finalize(ba_val);
         if (ba_val.size() == 0)
         {

common/interpreter/wawaka_wasm/WawakaInterpreter.cpp

@@ -86,6 +86,36 @@ int wasm_printer(const char *msg)
 // Should be defined in WasmExtensions.cpp
 extern bool InitializeNativeSymbols(RuntimeInitArgs& init_args);
 
+// XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
+bool WawakaInterpreter::close_kv_store(void)
+{
+    bool result = true;
+    ByteArray statehash;
+
+    for (size_t i = 1; i < kv_store_pool_.size(); i++)
+    {
+        if (kv_store_pool_[i] == NULL)
+            continue;
+
+        result = false;
+
+        SAFE_LOG(PDO_LOG_WARNING, "kv_store_pool_ slot not finalized: %u", i);
+        try {
+            kv_store_pool_[i]->Finalize(statehash);
+            delete kv_store_pool_[i];
+            kv_store_pool_[i] = NULL;
+        }
+        catch (std::exception& e)
+        {
+            // Only log the exception here
+            SAFE_LOG(PDO_LOG_ERROR, "Exception occured while finalizing outstanding state pool: %s", e.what());
+        }
+    }
+
+    // returns false if there were any outstanding kv stores
+    return result;
+}
+
 // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 void WawakaInterpreter::parse_response_string(
     int32 response_app,
@@ -403,12 +433,8 @@ void WawakaInterpreter::create_initial_contract_state(
     std::map<std::string,std::string> outDependencies;
     parse_response_string(response_app, outMessageResult, outStateChangedFlag, outDependencies);
 
-    // We could throw an exception if the store is not finalized
-    // or we could just finalize and throw away the block id, which
-    // effectively loses access to the kv store, seems like throwing
-    // an exception is the right idea
-    for (size_t i = 1; i < kv_store_pool_.size(); i++)
-        pe::ThrowIf<pe::RuntimeError>(kv_store_pool_[i] != NULL, "failed to close contract KV store");
+    // for the moment, not closing the kv store is bad practice but not an error
+    (void) close_kv_store();
 
     // this should be in finally... later...
     wasm_runtime_set_custom_data(wasm_module_inst_, NULL);
@@ -443,12 +469,8 @@ void WawakaInterpreter::send_message_to_contract(
     int32 response_app = evaluate_function(inMessage.Message, env);
     parse_response_string(response_app, outMessageResult, outStateChangedFlag, outDependencies);
 
-    // We could throw an exception if the store is not finalized
-    // or we could just finalize and throw away the block id, which
-    // effectively loses access to the kv store, seems like throwing
-    // an exception is the right idea
-    for (size_t i = 1; i < kv_store_pool_.size(); i++)
-        pe::ThrowIf<pe::RuntimeError>(kv_store_pool_[i] != NULL, "failed to close contract KV store");
+    // for the moment, not closing the kv store is bad practice but not an error
+    (void) close_kv_store();
 
     // this should be in finally... later...
     wasm_runtime_set_custom_data(wasm_module_inst_, NULL);

common/interpreter/wawaka_wasm/WawakaInterpreter.h

@@ -51,6 +51,8 @@ class WawakaInterpreter : public pc::ContractInterpreter
     wasm_module_inst_t wasm_module_inst_ = NULL;
     wasm_exec_env_t wasm_exec_env_ = NULL;
 
+    bool close_kv_store(void);
+
     void parse_response_string(
         int32 response_app,
         std::string& outResult,