If an algorithm cache lookup fails, index the newly created prototype object

under the name that the algorithm was originally requested by. This enables
proper caching for algorithm names which deref_alias fails to fully dereference
such as "HMAC(SHA-1)". The previous code had two major problems with names of
that type, firstly that the cache was effectively bypassed due to all prototype
objects in Algorithm_Cache_Impl being indexed by their canonical names rather
than the alias that they were requested under, and that there existed a race
condition where a prototype object might be deleted while in use in multithreaded
code.

The downside of this change is that using multiple names to refer to a single
algorithm causes multiple prototype objects to be created, one for each name
that is in use. However the memory overhead of this should be fairly minimal
and given the severity of the race condition this seems like a worthwhile tradeoff.

A more complete fix would be to fix deref_alias to properly derference all alias
names. That fix would be complimentary with this change in that if deref_alias
handled all names properly there would be a single prototype object and there
would then be no additional memory overhead to the cache.

2 files changed, 17 insertions, 12 deletions

diff --git a/include/engine.h b/include/engine.h
index 9009f01fb..76147f347 100644
--- a/include/engine.h
+++ b/include/engine.h
@@ -28,7 +28,7 @@ class Engine
          {
          public:
             virtual T* get(const std::string&) const = 0;
-            virtual void add(T* algo) const = 0;
+            virtual void add(T* algo, const std::string& = "") const = 0;
             virtual ~Algorithm_Cache() {}
          };
 
@@ -75,14 +75,18 @@ class Engine
          find_bc_pad(const std::string&) const;
 
       template<typename T>
-      T* lookup_algo(const Algorithm_Cache<T>* cache,
-                     const std::string& name,
-                     const Engine* engine,
-                     T* (Engine::*find)(const std::string& name) const) const
+      const T* lookup_algo(const Algorithm_Cache<T>* cache,
+                           const std::string& name,
+                           const Engine* engine,
+                           T* (Engine::*find)(const std::string&) const) const
          {
          T* algo = cache->get(name);
          if(!algo)
-            cache->add(algo = (engine->*find)(name));
+            {
+            algo = (engine->*find)(name);
+            if(algo)
+               cache->add(algo, name);
+            }
          return algo;
          }
 
diff --git a/src/eng_base.cpp b/src/eng_base.cpp
index a726fd762..c8aa4430c 100644
--- a/src/eng_base.cpp
+++ b/src/eng_base.cpp
@@ -25,17 +25,19 @@ class Algorithm_Cache_Impl : public Engine::Algorithm_Cache<T>
          return search_map(mappings, name);
          }
 
-      void add(T* algo) const
+      void add(T* algo, const std::string& index_name = "") const
          {
          if(!algo)
             return;
 
          Mutex_Holder lock(mutex);
 
-         const std::string algo_name = algo->name();
-         if(mappings.find(algo_name) != mappings.end())
-            delete mappings[algo_name];
-         mappings[algo_name] = algo;
+         const std::string name =
+            (index_name != "" ? index_name : algo->name());
+
+         if(mappings.find(name) != mappings.end())
+            delete mappings[name];
+         mappings[name] = algo;
          }
 
       Algorithm_Cache_Impl()
@@ -59,7 +61,6 @@ class Algorithm_Cache_Impl : public Engine::Algorithm_Cache<T>
       mutable std::map<std::string, T*> mappings;
    };
 
-
 }
 
 /*************************************************