From 8ccf4874e39a9814aaaae50b0483d025f1494cc7 Mon Sep 17 00:00:00 2001 From: lloyd Date: Sat, 24 Jan 2015 16:14:30 +0000 Subject: Handle repeated initializations of the library better and deal with initializations across multiple threads safely. --- src/lib/libstate/global_state.cpp | 55 ++++++++++++++++++--------------------- src/lib/libstate/init.cpp | 49 ++++++++++++++++------------------ src/lib/libstate/init.h | 27 ++++--------------- src/lib/libstate/libstate.cpp | 4 +++ src/lib/libstate/libstate.h | 2 ++ 5 files changed, 60 insertions(+), 77 deletions(-) (limited to 'src/lib') 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 #include +#include +#include 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 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 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 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 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 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; -- cgit v1.2.3