From 2f6f59e228f05a95e231ee765914a29072371ec1 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Fri, 21 Feb 2014 08:47:24 +0100 Subject: [PATCH] fixed startup race condition --- arangod/Cluster/ApplicationCluster.cpp | 8 ++--- arangod/Cluster/ClusterComm.cpp | 43 +++++++++++++++++--------- arangod/Cluster/ClusterComm.h | 6 ++++ arangod/Cluster/ClusterInfo.cpp | 14 ++++++--- arangod/Cluster/ClusterInfo.h | 6 ++++ 5 files changed, 54 insertions(+), 23 deletions(-) diff --git a/arangod/Cluster/ApplicationCluster.cpp b/arangod/Cluster/ApplicationCluster.cpp index a44301e603..86667bbcf1 100644 --- a/arangod/Cluster/ApplicationCluster.cpp +++ b/arangod/Cluster/ApplicationCluster.cpp @@ -155,14 +155,12 @@ bool ApplicationCluster::prepare () { } } + // initialise ClusterInfo library + ClusterInfo::initialise(); + // initialise ClusterComm library ClusterComm::initialise(); - // initialise cluster info library - ClusterInfo::instance(); - - usleep(1000); - return true; } diff --git a/arangod/Cluster/ClusterComm.cpp b/arangod/Cluster/ClusterComm.cpp index 58a4499e5f..75e133880a 100644 --- a/arangod/Cluster/ClusterComm.cpp +++ b/arangod/Cluster/ClusterComm.cpp @@ -59,14 +59,8 @@ void triagens::arango::ClusterCommRestCallback(string& coordinator, /// @brief ClusterComm constructor //////////////////////////////////////////////////////////////////////////////// -ClusterComm::ClusterComm () { - _backgroundThread = new ClusterCommThread(); - if (0 == _backgroundThread) { - LOG_FATAL_AND_EXIT("unable to start ClusterComm background thread"); - } - if (! _backgroundThread->init() || ! _backgroundThread->start()) { - LOG_FATAL_AND_EXIT("ClusterComm background thread does not work"); - } +ClusterComm::ClusterComm () : + _backgroundThread(0) { } //////////////////////////////////////////////////////////////////////////////// @@ -74,10 +68,13 @@ ClusterComm::ClusterComm () { //////////////////////////////////////////////////////////////////////////////// ClusterComm::~ClusterComm () { - _backgroundThread->stop(); - _backgroundThread->shutdown(); - delete _backgroundThread; - _backgroundThread = 0; + if (_backgroundThread != 0) { + _backgroundThread->stop(); + _backgroundThread->shutdown(); + delete _backgroundThread; + _backgroundThread = 0; + } + cleanupAllQueues(); } @@ -100,12 +97,30 @@ ClusterComm* ClusterComm::instance () { } //////////////////////////////////////////////////////////////////////////////// -/// @brief only used to trigger creation +/// @brief initialise the cluster comm singleton object //////////////////////////////////////////////////////////////////////////////// void ClusterComm::initialise () { assert(_theinstance == 0); - _theinstance = new ClusterComm( ); // this now happens exactly once + _theinstance = new ClusterComm(); // this now happens exactly once + + _theinstance->startBackgroundThread(); +} + +//////////////////////////////////////////////////////////////////////////////// +/// @brief start the communication background thread +//////////////////////////////////////////////////////////////////////////////// + +void ClusterComm::startBackgroundThread () { + _backgroundThread = new ClusterCommThread(); + + if (0 == _backgroundThread) { + LOG_FATAL_AND_EXIT("unable to start ClusterComm background thread"); + } + + if (! _backgroundThread->init() || ! _backgroundThread->start()) { + LOG_FATAL_AND_EXIT("ClusterComm background thread does not work"); + } } //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/Cluster/ClusterComm.h b/arangod/Cluster/ClusterComm.h index 0b7f28df70..b9b9d2ff44 100644 --- a/arangod/Cluster/ClusterComm.h +++ b/arangod/Cluster/ClusterComm.h @@ -261,6 +261,12 @@ void ClusterCommRestCallback(string& coordinator, rest::HttpResponse* response); _theinstance = 0; } +//////////////////////////////////////////////////////////////////////////////// +/// @brief start the communication background thread +//////////////////////////////////////////////////////////////////////////////// + + void startBackgroundThread (); + //////////////////////////////////////////////////////////////////////////////// /// @brief submit an HTTP request to a shard asynchronously. //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/Cluster/ClusterInfo.cpp b/arangod/Cluster/ClusterInfo.cpp index 97a73c0174..d7ae881355 100644 --- a/arangod/Cluster/ClusterInfo.cpp +++ b/arangod/Cluster/ClusterInfo.cpp @@ -244,12 +244,19 @@ ClusterInfo* ClusterInfo::instance () { // This does not have to be thread-safe, because we guarantee that // this is called very early in the startup phase when there is still // a single thread. - if (0 == _theinstance) { - _theinstance = new ClusterInfo(); // this now happens exactly once - } + assert(_theinstance != 0); return _theinstance; } +//////////////////////////////////////////////////////////////////////////////// +/// @brief initialise the cluster info singleton object +//////////////////////////////////////////////////////////////////////////////// + +void ClusterInfo::initialise () { + assert(_theinstance == 0); + _theinstance = new ClusterInfo(); // this now happens exactly once +} + //////////////////////////////////////////////////////////////////////////////// /// @brief creates a cluster info object //////////////////////////////////////////////////////////////////////////////// @@ -2085,7 +2092,6 @@ int ClusterInfo::getResponsibleShard (CollectionID const& collectionID, return error; } - // Local Variables: // mode: outline-minor // outline-regexp: "^\\(/// @brief\\|/// {@inheritDoc}\\|/// @addtogroup\\|// --SECTION--\\|/// @\\}\\)" diff --git a/arangod/Cluster/ClusterInfo.h b/arangod/Cluster/ClusterInfo.h index 4ac2a7c0ba..b7d0109e42 100644 --- a/arangod/Cluster/ClusterInfo.h +++ b/arangod/Cluster/ClusterInfo.h @@ -720,6 +720,12 @@ namespace triagens { static ClusterInfo* instance (); +//////////////////////////////////////////////////////////////////////////////// +/// @brief initialise function to call once when still single-threaded +//////////////////////////////////////////////////////////////////////////////// + + static void initialise (); + //////////////////////////////////////////////////////////////////////////////// /// @brief cleanup function to call once when shutting down ////////////////////////////////////////////////////////////////////////////////