Handle repeated initializations of the library better and deal with

initializations across multiple threads safely.

5 files changed, 60 insertions, 77 deletions

diff --git a/src/lib/libstate/global_state.cpp b/src/lib/libstate/global_state.cpp
index 3fd202d5f..b9b755d87 100644
--- a/src/lib/libstate/global_state.cpp
+++ b/src/lib/libstate/global_state.cpp
@@ -1,28 +1,23 @@
 /*
 * Global State Management
-* (C) 2010 Jack Lloyd
+* (C) 2010,2015 Jack Lloyd
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
 
 #include <botan/global_state.h>
 #include <botan/libstate.h>
+#include <memory>
+#include <mutex>
 
 namespace Botan {
 
-/*
-* @todo There should probably be a lock to avoid racy manipulation
-* of the state among different threads
-*/
-
 namespace Global_State_Management {
 
-/*
-* Botan's global state
-*/
 namespace {
 
-Library_State* global_lib_state = nullptr;
+std::mutex g_lib_state_mutex;
+std::unique_ptr<Library_State> g_lib_state;
 
 }
 
@@ -31,41 +26,42 @@ Library_State* global_lib_state = nullptr;
 */
 Library_State& global_state()
    {
+   // @todo use double checked locking? (Is this safe in C++11 mm?)
+   std::lock_guard<std::mutex> lock(g_lib_state_mutex);
+
    /* Lazy initialization. Botan still needs to be deinitialized later
       on or memory might leak.
    */
-   if(!global_lib_state)
+   if(!g_lib_state)
       {
-      global_lib_state = new Library_State;
-      global_lib_state->initialize();
+      g_lib_state.reset(new Library_State);
+      g_lib_state->initialize();
       }
 
-   return (*global_lib_state);
+   return (*g_lib_state);
    }
 
 /*
 * Set a new global state object
 */
-void set_global_state(Library_State* new_state)
+void set_global_state(Library_State* state)
    {
-   delete swap_global_state(new_state);
+   std::lock_guard<std::mutex> lock(g_lib_state_mutex);
+   g_lib_state.reset(state);
    }
 
 /*
 * Set a new global state object unless one already existed
 */
-bool set_global_state_unless_set(Library_State* new_state)
+bool set_global_state_unless_set(Library_State* state)
    {
-   if(global_lib_state)
-      {
-      delete new_state;
+   std::lock_guard<std::mutex> lock(g_lib_state_mutex);
+
+   if(g_lib_state)
       return false;
-      }
-   else
-      {
-      delete swap_global_state(new_state);
-      return true;
-      }
+
+   g_lib_state.reset(state);
+   return true;
    }
 
 /*
@@ -73,8 +69,9 @@ bool set_global_state_unless_set(Library_State* new_state)
 */
 Library_State* swap_global_state(Library_State* new_state)
    {
-   Library_State* old_state = global_lib_state;
-   global_lib_state = new_state;
+   std::lock_guard<std::mutex> lock(g_lib_state_mutex);
+   Library_State* old_state = g_lib_state.release();
+   g_lib_state.reset(new_state);
    return old_state;
    }
 
@@ -83,7 +80,7 @@ Library_State* swap_global_state(Library_State* new_state)
 */
 bool global_state_exists()
    {
-   return (global_lib_state != nullptr);
+   return (g_lib_state != nullptr);
    }
 
 }
diff --git a/src/lib/libstate/init.cpp b/src/lib/libstate/init.cpp
index 09ecca5a1..6155b3bd2 100644
--- a/src/lib/libstate/init.cpp
+++ b/src/lib/libstate/init.cpp
@@ -1,6 +1,6 @@
 /*
-* Default Initialization Function
-* (C) 1999-2009 Jack Lloyd
+* Library initialization
+* (C) 1999-2009.2015 Jack Lloyd
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -11,37 +11,34 @@
 
 namespace Botan {
 
-/*
-* Library Initialization
-*/
-void LibraryInitializer::initialize(const std::string&)
+LibraryInitializer::LibraryInitializer()
    {
+   /*
+   This two stage initialization process is because Library_State's
+   constructor will implicitly refer to global state through the
+   allocators and so forth, so global_state() has to be a valid
+   reference before initialize() can be called. Yeah, gross.
+   */
+   m_owned = Global_State_Management::set_global_state_unless_set(new Library_State);
 
-   try
+   if(m_owned)
       {
-      /*
-      This two stage initialization process is because Library_State's
-      constructor will implicitly refer to global state through the
-      allocators and so forth, so global_state() has to be a valid
-      reference before initialize() can be called. Yeah, gross.
-      */
-      Global_State_Management::set_global_state(new Library_State);
-
-      global_state().initialize();
-      }
-   catch(...)
-      {
-      deinitialize();
-      throw;
+      try
+         {
+         global_state().initialize();
+         }
+      catch(...)
+         {
+         Global_State_Management::set_global_state(nullptr);
+         throw;
+         }
       }
    }
 
-/*
-* Library Shutdown
-*/
-void LibraryInitializer::deinitialize()
+LibraryInitializer::~LibraryInitializer()
    {
-   Global_State_Management::set_global_state(nullptr);
+   if(m_owned)
+      Global_State_Management::set_global_state(nullptr);
    }
 
 }
diff --git a/src/lib/libstate/init.h b/src/lib/libstate/init.h
index 6f061df03..46bcc66fa 100644
--- a/src/lib/libstate/init.h
+++ b/src/lib/libstate/init.h
@@ -15,32 +15,15 @@ namespace Botan {
 
 /**
 * This class represents the Library Initialization/Shutdown Object. It
-* has to exceed the lifetime of any Botan object used in an
-* application.  You can call initialize/deinitialize or use
-* LibraryInitializer in the RAII style.
+* has to exceed the lifetime of any Botan object used in an application.
 */
 class BOTAN_DLL LibraryInitializer
    {
    public:
-      /**
-      * Initialize the library
-      * @param options a string listing initialization options
-      */
-      static void initialize(const std::string& options = "");
-
-      /**
-      * Shutdown the library
-      */
-      static void deinitialize();
-
-      /**
-      * Initialize the library
-      * @param options a string listing initialization options
-      */
-      LibraryInitializer(const std::string& options = "")
-         { LibraryInitializer::initialize(options); }
-
-      ~LibraryInitializer() { LibraryInitializer::deinitialize(); }
+      LibraryInitializer();
+      ~LibraryInitializer();
+   private:
+      bool m_owned;
    };
 
 }
diff --git a/src/lib/libstate/libstate.cpp b/src/lib/libstate/libstate.cpp
index 87cffd318..6ce9c7eae 100644
--- a/src/lib/libstate/libstate.cpp
+++ b/src/lib/libstate/libstate.cpp
@@ -58,6 +58,10 @@ RandomNumberGenerator& Library_State::global_rng()
    return *m_global_prng;
    }
 
+Library_State::~Library_State()
+   {
+   }
+
 void Library_State::initialize()
    {
    if(m_algorithm_factory.get())
diff --git a/src/lib/libstate/libstate.h b/src/lib/libstate/libstate.h
index a85fc7e5a..09884df24 100644
--- a/src/lib/libstate/libstate.h
+++ b/src/lib/libstate/libstate.h
@@ -26,6 +26,8 @@ class BOTAN_DLL Library_State
    public:
       Library_State() {}
 
+      ~Library_State();
+
       Library_State(const Library_State&) = delete;
       Library_State& operator=(const Library_State&) = delete;