Fix static_initializer: …

* Prevents double construction, invalid access
* Unit test works on MSVC and non MSVC

Builds/VisualStudio2013/RippleD.vcxproj

@@ -1147,6 +1147,8 @@
     </ClInclude>
     <ClInclude Include="..\..\src\beast\beast\utility\noexcept.h">
     </ClInclude>
+    <ClInclude Include="..\..\src\beast\beast\utility\static_initializer.h">
+    </ClInclude>
     <ClInclude Include="..\..\src\beast\beast\utility\tagged_integer.h">
     </ClInclude>
     <ClCompile Include="..\..\src\beast\beast\utility\tests\Journal.test.cpp">
@@ -1161,6 +1163,9 @@
     <ClCompile Include="..\..\src\beast\beast\utility\tests\empty_base_optimization.test.cpp">
       <ExcludedFromBuild>True</ExcludedFromBuild>
     </ClCompile>
+    <ClCompile Include="..\..\src\beast\beast\utility\tests\static_initializer.test.cpp">
+      <ExcludedFromBuild>True</ExcludedFromBuild>
+    </ClCompile>
     <ClCompile Include="..\..\src\beast\beast\utility\tests\tagged_integer.test.cpp">
       <ExcludedFromBuild>True</ExcludedFromBuild>
     </ClCompile>
@@ -2037,6 +2042,8 @@
     <ClCompile Include="..\..\src\ripple\module\app\ledger\BookListeners.cpp">
       <ExcludedFromBuild>True</ExcludedFromBuild>
     </ClCompile>
+    <ClInclude Include="..\..\src\ripple\module\app\ledger\BookListeners.h">
+    </ClInclude>
     <ClCompile Include="..\..\src\ripple\module\app\ledger\DirectoryEntryIterator.cpp">
       <ExcludedFromBuild>True</ExcludedFromBuild>
     </ClCompile>
@@ -2206,9 +2213,6 @@
     </ClCompile>
     <ClInclude Include="..\..\src\ripple\module\app\misc\Offer.h">
     </ClInclude>
-    <ClCompile Include="..\..\src\ripple\module\app\misc\OrderBook.cpp">
-      <ExcludedFromBuild>True</ExcludedFromBuild>
-    </ClCompile>
     <ClInclude Include="..\..\src\ripple\module\app\misc\OrderBook.h">
     </ClInclude>
     <ClInclude Include="..\..\src\ripple\module\app\misc\PowResult.h">
@@ -3203,16 +3207,16 @@
     <ClInclude Include="..\..\src\ripple\peerfinder\sim\WrappedSink.h">
     </ClInclude>
     <CustomBuild Include="..\..\src\ripple\proto\ripple.proto">
-      <FileType>Document</FileType>
-      <Command Condition="'$(Configuration)|$(Platform)'=='release|x64'">protoc --cpp_out=..\..\build\proto --proto_path=%(RelativeDir) %(Identity)</Command>
-      <Outputs Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\build\proto\ripple.pb.h;..\..\build\proto\ripple.pb.cc</Outputs>
-      <Message Condition="'$(Configuration)|$(Platform)'=='release|x64'">protoc --cpp_out=..\..\build\proto --proto_path=%(RelativeDir) %(Identity)</Message>
-      <LinkObjects Condition="'$(Configuration)|$(Platform)'=='release|x64'">false</LinkObjects>
       <FileType>Document</FileType>
       <Command Condition="'$(Configuration)|$(Platform)'=='debug|x64'">protoc --cpp_out=..\..\build\proto --proto_path=%(RelativeDir) %(Identity)</Command>
       <Outputs Condition="'$(Configuration)|$(Platform)'=='debug|x64'">..\..\build\proto\ripple.pb.h;..\..\build\proto\ripple.pb.cc</Outputs>
       <Message Condition="'$(Configuration)|$(Platform)'=='debug|x64'">protoc --cpp_out=..\..\build\proto --proto_path=%(RelativeDir) %(Identity)</Message>
       <LinkObjects Condition="'$(Configuration)|$(Platform)'=='debug|x64'">false</LinkObjects>
+      <FileType>Document</FileType>
+      <Command Condition="'$(Configuration)|$(Platform)'=='release|x64'">protoc --cpp_out=..\..\build\proto --proto_path=%(RelativeDir) %(Identity)</Command>
+      <Outputs Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\build\proto\ripple.pb.h;..\..\build\proto\ripple.pb.cc</Outputs>
+      <Message Condition="'$(Configuration)|$(Platform)'=='release|x64'">protoc --cpp_out=..\..\build\proto --proto_path=%(RelativeDir) %(Identity)</Message>
+      <LinkObjects Condition="'$(Configuration)|$(Platform)'=='release|x64'">false</LinkObjects>
     </CustomBuild>
     <ClInclude Include="..\..\src\ripple\radmap\api\BasicFullBelowCache.h">
     </ClInclude>
@@ -3447,8 +3451,8 @@
     <ClInclude Include="..\..\src\ripple\unity\http.h">
     </ClInclude>
     <ClCompile Include="..\..\src\ripple\unity\hyperleveldb.cpp">
-      <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\src\hyperleveldb;..\..\src\snappy\config;..\..\src\snappy\snappy;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='debug|x64'">..\..\src\hyperleveldb;..\..\src\snappy\config;..\..\src\snappy\snappy;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\src\hyperleveldb;..\..\src\snappy\config;..\..\src\snappy\snappy;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
     </ClCompile>
     <ClInclude Include="..\..\src\ripple\unity\hyperleveldb.h">
     </ClInclude>
@@ -3457,8 +3461,8 @@
     <ClInclude Include="..\..\src\ripple\unity\json.h">
     </ClInclude>
     <ClCompile Include="..\..\src\ripple\unity\leveldb.cpp">
-      <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\src\leveldb;..\..\src\leveldb\include;..\..\src\snappy\config;..\..\src\snappy\snappy;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='debug|x64'">..\..\src\leveldb;..\..\src\leveldb\include;..\..\src\snappy\config;..\..\src\snappy\snappy;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\src\leveldb;..\..\src\leveldb\include;..\..\src\snappy\config;..\..\src\snappy\snappy;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
     </ClCompile>
     <ClInclude Include="..\..\src\ripple\unity\leveldb.h">
     </ClInclude>
@@ -3467,8 +3471,8 @@
     <ClInclude Include="..\..\src\ripple\unity\net.h">
     </ClInclude>
     <ClCompile Include="..\..\src\ripple\unity\nodestore.cpp">
-      <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\src\leveldb\include;..\..\src\rocksdb\include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='debug|x64'">..\..\src\leveldb\include;..\..\src\rocksdb\include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\src\leveldb\include;..\..\src\rocksdb\include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
     </ClCompile>
     <ClCompile Include="..\..\src\ripple\unity\overlay.cpp">
     </ClCompile>
@@ -3489,8 +3493,8 @@
     <ClCompile Include="..\..\src\ripple\unity\ripple.proto.cpp">
     </ClCompile>
     <ClCompile Include="..\..\src\ripple\unity\rocksdb.cpp">
-      <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\src\rocksdb;..\..\src\rocksdb\include;..\..\src\snappy\config;..\..\src\snappy\snappy;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='debug|x64'">..\..\src\rocksdb;..\..\src\rocksdb\include;..\..\src\snappy\config;..\..\src\snappy\snappy;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\src\rocksdb;..\..\src\rocksdb\include;..\..\src\snappy\config;..\..\src\snappy\snappy;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
     </ClCompile>
     <ClInclude Include="..\..\src\ripple\unity\rocksdb.h">
     </ClInclude>
@@ -3503,8 +3507,8 @@
     <ClInclude Include="..\..\src\ripple\unity\sitefiles.h">
     </ClInclude>
     <ClCompile Include="..\..\src\ripple\unity\snappy.cpp">
-      <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\src\snappy\config;..\..\src\snappy\snappy;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='debug|x64'">..\..\src\snappy\config;..\..\src\snappy\snappy;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='release|x64'">..\..\src\snappy\config;..\..\src\snappy\snappy;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
     </ClCompile>
     <ClCompile Include="..\..\src\ripple\unity\sslutil.cpp">
     </ClCompile>

Builds/VisualStudio2013/RippleD.vcxproj.filters

@@ -1926,6 +1926,9 @@
     <ClInclude Include="..\..\src\beast\beast\utility\noexcept.h">
       <Filter>beast\utility</Filter>
     </ClInclude>
+    <ClInclude Include="..\..\src\beast\beast\utility\static_initializer.h">
+      <Filter>beast\utility</Filter>
+    </ClInclude>
     <ClInclude Include="..\..\src\beast\beast\utility\tagged_integer.h">
       <Filter>beast\utility</Filter>
     </ClInclude>
@@ -1941,6 +1944,9 @@
     <ClCompile Include="..\..\src\beast\beast\utility\tests\empty_base_optimization.test.cpp">
       <Filter>beast\utility\tests</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\src\beast\beast\utility\tests\static_initializer.test.cpp">
+      <Filter>beast\utility\tests</Filter>
+    </ClCompile>
     <ClCompile Include="..\..\src\beast\beast\utility\tests\tagged_integer.test.cpp">
       <Filter>beast\utility\tests</Filter>
     </ClCompile>
@@ -3030,6 +3036,9 @@
     <ClCompile Include="..\..\src\ripple\module\app\ledger\BookListeners.cpp">
       <Filter>ripple\module\app\ledger</Filter>
     </ClCompile>
+    <ClInclude Include="..\..\src\ripple\module\app\ledger\BookListeners.h">
+      <Filter>ripple\module\app\ledger</Filter>
+    </ClInclude>
     <ClCompile Include="..\..\src\ripple\module\app\ledger\DirectoryEntryIterator.cpp">
       <Filter>ripple\module\app\ledger</Filter>
     </ClCompile>
@@ -3234,9 +3243,6 @@
     <ClInclude Include="..\..\src\ripple\module\app\misc\Offer.h">
       <Filter>ripple\module\app\misc</Filter>
     </ClInclude>
-    <ClCompile Include="..\..\src\ripple\module\app\misc\OrderBook.cpp">
-      <Filter>ripple\module\app\misc</Filter>
-    </ClCompile>
     <ClInclude Include="..\..\src\ripple\module\app\misc\OrderBook.h">
       <Filter>ripple\module\app\misc</Filter>
     </ClInclude>

src/beast/beast/utility/Utility.unity.cpp

@@ -32,5 +32,6 @@
 #include <beast/utility/tests/bassert.test.cpp>
 #include <beast/utility/tests/empty_base_optimization.test.cpp>
 #include <beast/utility/tests/Journal.test.cpp>
+#include <beast/utility/tests/static_initializer.test.cpp>
 #include <beast/utility/tests/tagged_integer.test.cpp>
 #include <beast/utility/tests/Zero.test.cpp>

src/beast/beast/utility/static_initializer.h

@@ -20,14 +20,16 @@
 #ifndef BEAST_UTILITY_STATIC_INITIALIZER_H_INCLUDED
 #define BEAST_UTILITY_STATIC_INITIALIZER_H_INCLUDED
 
-#ifdef _MSC_VER
-
 #include <beast/utility/noexcept.h>
-#include <atomic>
+#include <utility>
-#include <chrono>
+
+#ifdef _MSC_VER
+#include <cassert>
 #include <new>
 #include <thread>
 #include <type_traits>
+#include <intrin.h>
+#endif
 
 namespace beast {
 
@@ -44,7 +46,129 @@ namespace beast {
     }
     @endcode
 */
-template <class T, class Tag = void>
+#ifdef _MSC_VER
+template <
+    class T,
+    class Tag = void
+>
+class static_initializer
+{
+private:
+    struct data_t
+    {
+        //  0 = unconstructed
+        //  1 = constructing
+        //  2 = constructed
+        long volatile state;
+
+        typename std::aligned_storage <sizeof(T),
+            std::alignment_of <T>::value>::type storage;
+    };
+
+    struct destroyer
+    {
+        T* t_;
+        explicit destroyer (T* t) : t_(t) { }
+        ~destroyer() { t_->~T(); }
+    };
+
+    static
+    data_t&
+    data() noexcept;
+
+public:
+    template <class... Args>
+    explicit static_initializer (Args&&... args);
+
+    T&
+    get() noexcept;
+
+    T&
+    operator*() noexcept
+    {
+        return get();
+    }
+
+    T*
+    operator->() noexcept
+    {
+        return &get();
+    }
+};
+
+//------------------------------------------------------------------------------
+
+template <class T, class Tag>
+auto
+static_initializer <T, Tag>::data() noexcept ->
+    data_t&
+{
+    static data_t _; // zero-initialized
+    return _;
+}
+
+template <class T, class Tag>
+template <class... Args>
+static_initializer <T, Tag>::static_initializer (Args&&... args)
+{
+    data_t& _(data());
+
+    // Double Checked Locking Pattern
+
+    if (_.state != 2)
+    {
+        T* const t (reinterpret_cast<T*>(&_.storage));
+
+        for(;;)
+        {
+            long prev;
+            prev = InterlockedCompareExchange(&_.state, 1, 0);
+            if (prev == 0)
+            {
+                try
+                {
+                    ::new(t) T (std::forward<Args>(args)...);                   
+                    static destroyer on_exit (t);
+                    InterlockedIncrement(&_.state);
+                }
+                catch(...)
+                {
+                    // Constructors that throw exceptions are unsupported
+                    std::terminate();
+                }
+            }
+            else if (prev == 1)
+            {
+                std::this_thread::yield();
+            }
+            else
+            {
+                assert(prev == 2);
+                break;
+            }
+        }
+    }
+}
+
+template <class T, class Tag>
+T&
+static_initializer <T, Tag>::get() noexcept
+{
+    data_t& _(data());
+    for(;;)
+    {
+        if (_.state == 2)
+            break;
+        std::this_thread::yield();
+    }
+    return *reinterpret_cast<T*>(&_.storage);
+}
+
+#else
+template <
+    class T,
+    class Tag = void
+>
 class static_initializer
 {
 private:
@@ -52,53 +176,8 @@ private:
 
 public:
     template <class... Args>
-    static_initializer(Args&&... args)
+    explicit
-    {
+    static_initializer (Args&&... args);
-        static std::aligned_storage <sizeof(T),
-            std::alignment_of <T>::value>::type storage;
-        instance_ = reinterpret_cast<T*>(&storage);
-
-        // double checked lock
-        static bool volatile initialized;
-        if (! initialized)
-        {
-            static std::atomic_flag lock;
-            while (lock.test_and_set())
-                std::this_thread::sleep_for (
-                    std::chrono::milliseconds(10));
-            if (! initialized)
-            {
-                try
-                {
-                    ::new(instance_) T(std::forward<Args>(args)...);
-                    
-                    struct destroyer
-                    {
-                        T* t_;
-
-                        destroyer (T* t)
-                            : t_(t)
-                        {
-                        }
-
-                        ~destroyer()
-                        {
-                            t_->~T();
-                        }
-                    };
-
-                    static destroyer on_exit (instance_);
-                }
-                catch(...)
-                {
-                    lock.clear();
-                    throw;
-                }
-                initialized = true;
-            }
-            lock.clear();
-        }
-    }
 
     T&
     get() noexcept
@@ -109,12 +188,26 @@ public:
     T&
     operator*() noexcept
     {
-        return *instance_;
+        return get();
+    }
+
+    T*
+    operator->() noexcept
+    {
+        return &get();
     }
 };
 
+template <class T, class Tag>
+template <class... Args>
+static_initializer <T, Tag>::static_initializer (Args&&... args)
+{
+    static T t (std::forward<Args>(args)...);
+    instance_ = &t;
 }
 
 #endif
 
+}
+
 #endif

src/beast/beast/utility/tests/static_initializer.test.cpp

@@ -0,0 +1,216 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of Beast: https://github.com/vinniefalco/Beast
+    Copyright 2013, Vinnie Falco <vinnie.falco@gmail.com>
+
+    Permission to use, copy, modify, and/or distribute this software for any
+    purpose  with  or without fee is hereby granted, provided that the above
+    copyright notice and this permission notice appear in all copies.
+
+    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
+    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+    ANY  SPECIAL ,  DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
+    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+*/
+//==============================================================================
+
+#include <beast/utility/static_initializer.h>
+#include <beast/unit_test/suite.h>
+#include <atomic>
+#include <sstream>
+#include <thread>
+#include <utility>
+
+namespace beast {
+
+static_assert(__alignof(long) >= 4, "");
+
+class static_initializer_test : public unit_test::suite
+{
+public:
+    // Used to create separate instances for each test
+    struct cxx11_tag { };
+    struct beast_tag { };
+    template <std::size_t N, class Tag>
+    struct Case
+    {
+        enum { count = N };
+        typedef Tag type;
+    };
+
+    struct Counts
+    {
+        Counts()
+            : calls (0)
+            , constructed (0)
+            , access (0)
+        {
+        }
+
+        // number of calls to the constructor
+        std::atomic <long> calls;
+
+        // incremented after construction completes
+        std::atomic <long> constructed;
+
+        // increment when class is accessed before construction
+        std::atomic <long> access;
+    };
+
+    /*  This testing singleton detects two conditions:
+        1. Being accessed before getting fully constructed
+        2. Getting constructed twice
+    */
+    template <class Tag>
+    class Test;
+
+    template <class Function>
+    static
+    void
+    run_many (std::size_t n, Function f);
+
+    template <class Tag>
+    void
+    test (cxx11_tag);
+
+    template <class Tag>
+    void
+    test (beast_tag);
+
+    template <class Tag>
+    void
+    test();
+
+    void
+    run ();
+};
+
+//------------------------------------------------------------------------------
+
+template <class Tag>
+class static_initializer_test::Test
+{
+public:
+    explicit
+    Test (Counts& counts);
+
+    void
+    operator() (Counts& counts);
+};
+
+template <class Tag>
+static_initializer_test::Test<Tag>::Test (Counts& counts)
+{
+    ++counts.calls;
+    std::this_thread::sleep_for (std::chrono::milliseconds (10));
+    ++counts.constructed;
+}
+
+template <class Tag>
+void
+static_initializer_test::Test<Tag>::operator() (Counts& counts)
+{
+    if (! counts.constructed)
+        ++counts.access;
+}
+
+//------------------------------------------------------------------------------
+
+template <class Function>
+void
+static_initializer_test::run_many (std::size_t n, Function f)
+{
+    std::atomic <bool> start (false);
+    std::vector <std::thread> threads;
+
+    threads.reserve (n);
+
+    for (auto i (n); i-- ;)
+    {
+        threads.emplace_back([&]()
+        {
+            while (! start.load())
+                std::this_thread::yield();
+            f();
+        });
+    }
+    std::this_thread::sleep_for (std::chrono::milliseconds (10));
+    std::this_thread::yield();
+    start.store (true);
+    for (auto& thread : threads)
+        thread.join();
+}
+
+template <class Tag>
+void
+static_initializer_test::test (cxx11_tag)
+{
+    testcase << "cxx11 " << Tag::count << " threads";
+
+    Counts counts;
+
+    run_many (Tag::count, [&]()
+    {
+        static Test <Tag> t (counts);
+        t(counts);
+    });
+
+#ifdef _MSC_VER
+    // Visual Studio 2013 and earlier can exhibit both double
+    // construction, and access before construction when function
+    // local statics are initialized concurrently.
+    //
+    expect (counts.constructed > 1 || counts.access > 0);
+
+#else
+    expect (counts.constructed == 1 && counts.access == 0);
+
+#endif
+}
+
+template <class Tag>
+void
+static_initializer_test::test (beast_tag)
+{
+    testcase << "beast " << Tag::count << " threads";
+
+    Counts counts;
+
+    run_many (Tag::count, [&counts]()
+    {
+        static static_initializer <Test <Tag>> t (counts);
+        (*t)(counts);
+    });
+
+    expect (counts.constructed == 1 && counts.access == 0);
+}
+
+template <class Tag>
+void
+static_initializer_test::test()
+{
+    test <Tag> (typename Tag::type {});
+}
+
+void
+static_initializer_test::run ()
+{
+    test <Case<  4, cxx11_tag>> ();
+    test <Case< 16, cxx11_tag>> ();
+    test <Case< 64, cxx11_tag>> ();
+    test <Case<256, cxx11_tag>> ();
+
+    test <Case<  4, beast_tag>> ();
+    test <Case< 16, beast_tag>> ();
+    test <Case< 64, beast_tag>> ();
+    test <Case<256, beast_tag>> ();
+}
+
+//------------------------------------------------------------------------------
+
+BEAST_DEFINE_TESTSUITE(static_initializer,utility,beast);
+
+}

src/ripple/module/app/ledger/OrderBookDB.cpp

@@ -33,9 +33,9 @@ void OrderBookDB::invalidate ()
 
 void OrderBookDB::setup (Ledger::ref ledger)
 {
-    auto seq = ledger->getLedgerSeq ();
     {
         ScopedLockType sl (mLock);
+        auto seq = ledger->getLedgerSeq ();
 
         // Do a full update every 256 ledgers
         if (mSeq != 0)