Fixes for Service API hang on exit during unit tests

src/ripple_app/main/Application.cpp

@@ -13,7 +13,7 @@ static bool volatile doShutdown = false;
 
 // VFALCO NOTE This is temporary, until I refactor LogPartition
 //            and LogJournal::get() to take a string
-//                  
+//
 class ApplicationLog;
 template <> char const* LogPartition::getPartitionName <ApplicationLog> () { return "Application"; }
 class ValidatorsLog;
@@ -122,6 +122,7 @@ public:
 
     ~ApplicationImp ()
     {
+        serviceStop();
         //stop ();
 
         // Why is this needed here?
@@ -633,7 +634,7 @@ public:
         m_sweepTimer.cancel();
 
         mShutdown = true;
-        
+
         // This stalls for a long time
         //m_nodeStore = nullptr;
 
@@ -649,12 +650,6 @@ public:
     void run ()
     {
         {
-            // VFALCO TODO The unit tests crash if we try to
-            //             run these threads in the IoService constructor
-            //             so this hack makes them start later.
-            //
-            m_mainIoPool.runAsync ();
-
             if (!getConfig ().RUN_STANDALONE)
             {
                 // VFALCO NOTE This seems unnecessary. If we properly refactor the load
@@ -722,7 +717,7 @@ public:
         //
         m_stop.signal();
     }
-    
+
     void onDeadlineTimer (DeadlineTimer& timer)
     {
         if (timer == m_sweepTimer)

src/ripple_app/main/IoServicePool.cpp

@@ -15,13 +15,11 @@ public:
         , m_owner (owner)
         , m_service (service)
     {
-        //startThread ();
+        startThread ();
     }
 
     ~ServiceThread ()
     {
-        m_service.stop ();
-
         // block until thread exits
         stopThread ();
     }
@@ -50,7 +48,6 @@ IoServicePool::IoServicePool (Service& parent, String const& name, int numberOfT
     , m_name (name)
     , m_service (numberOfThreads)
     , m_work (boost::ref (m_service))
-    , m_running (false)
 {
     bassert (numberOfThreads > 0);
 
@@ -60,27 +57,15 @@ IoServicePool::IoServicePool (Service& parent, String const& name, int numberOfT
     {
         ++m_threadsRunning;
         m_threads.add (new ServiceThread (m_name, *this, m_service));
+        m_threads[i]->start ();
     }
 }
 
 IoServicePool::~IoServicePool ()
 {
-    // must have called runAsync()
-    bassert (m_running);
-
     // the dtor of m_threads will block until each thread exits.
 }
 
-void IoServicePool::runAsync ()
-{
-    // must not call twice
-    bassert (!m_running);
-    m_running = true;
-
-    for (int i = 0; i < m_threads.size (); ++i)
-        m_threads [i]->start ();
-}
-
 boost::asio::io_service& IoServicePool::getService ()
 {
     return m_service;

src/ripple_app/main/IoServicePool.h

@@ -14,8 +14,6 @@ public:
     IoServicePool (Service& parent, String const& name, int numberOfThreads);
     ~IoServicePool ();
 
-    void runAsync ();
-
     boost::asio::io_service& getService ();
     operator boost::asio::io_service& ();
 
@@ -32,7 +30,6 @@ private:
     boost::optional <boost::asio::io_service::work> m_work;
     OwnedArray <ServiceThread> m_threads;
     Atomic <int> m_threadsRunning;
-    bool m_running;
 };
 
 #endif

src/ripple_basics/utility/Service.cpp

@@ -18,7 +18,7 @@ Service::Service (char const* name, Service* parent)
     : m_name (name)
     , m_root (parent != nullptr)
     , m_child (this)
-     , m_calledServiceStop (false)
+    , m_calledServiceStop (false)
     , m_stopped (false)
     , m_childrenStopped (false)
 {

src/ripple_core/node/NodeStore.cpp

@@ -1077,7 +1077,9 @@ public:
         testBackend ("sophia", seedValue);
     #endif
 
+    /*
         testBackend ("sqlite", seedValue);
+    */
     }
 };
 
@@ -1114,8 +1116,8 @@ public:
         {
             ScopedService service ("test");
             ScopedPointer <NodeStore> src (NodeStore::New ("test", service, srcParams));
-
             storeBatch (*src, batch);
+            service.serviceStop();
         }
 
         NodeStore::Batch copy;
@@ -1141,6 +1143,8 @@ public:
 
             // Get the results of the import
             fetchCopyOfBatch (*dest, &copy, batch);
+
+            service.serviceStop();
         }
 
         // Canonicalize the source and destination batches
@@ -1204,6 +1208,8 @@ public:
                 fetchCopyOfBatch (*db, &copy, batch);
                 expect (areBatchesEqual (batch, copy), "Should be equal");
             }
+
+            service.serviceStop();
         }
 
         if (testPersistence)
@@ -1222,6 +1228,8 @@ public:
                 std::sort (batch.begin (), batch.end (), NodeObject::LessThan ());
                 std::sort (copy.begin (), copy.end (), NodeObject::LessThan ());
                 expect (areBatchesEqual (batch, copy), "Should be equal");
+
+                service.serviceStop();
             }
 
             if (useEphemeralDatabase)
@@ -1240,6 +1248,8 @@ public:
                 std::sort (batch.begin (), batch.end (), NodeObject::LessThan ());
                 std::sort (copy.begin (), copy.end (), NodeObject::LessThan ());
                 expect (areBatchesEqual (batch, copy), "Should be equal");
+
+                service.serviceStop();
             }
         }
     }