Merge pull request #702 from yahoo/balder/try-catch-the-components-construction

Balder/try catch the components construction

3 files changed, 30 insertions, 22 deletions

diff --git a/filedistribution/src/apps/filedistributor/filedistributor.cpp b/filedistribution/src/apps/filedistributor/filedistributor.cpp
index daef2099823..37f8d259258 100644
--- a/filedistribution/src/apps/filedistributor/filedistributor.cpp
+++ b/filedistribution/src/apps/filedistributor/filedistributor.cpp
@@ -56,7 +56,29 @@ class FileDistributor : public config::IFetcherCallback<ZookeepersConfig>,
         const std::shared_ptr<StateServerImpl> _stateServer;
 
     private:
-        std::unique_ptr<std::thread> _downloaderEventLoopThread;
+        class GuardedThread {
+        public:
+            GuardedThread(const GuardedThread &) = delete;
+            GuardedThread & operator = (const GuardedThread &) = delete;
+            GuardedThread(const std::shared_ptr<FileDownloader> & downloader) :
+                _downloader(downloader),
+                _thread([downloader=_downloader] () { downloader->runEventLoop(); })
+            { }
+            ~GuardedThread() {
+                _downloader->close();
+                if (_thread.joinable()) {
+                    _thread.join();
+                }
+                if ( !_downloader->drained() ) {
+                    LOG(error, "The filedownloader did not drain fully. We will just exit quickly and let a restart repair it for us.");
+                    std::quick_exit(67);
+                }
+            }
+        private:
+            std::shared_ptr<FileDownloader> _downloader;
+            std::thread                     _thread;
+        };
+        std::unique_ptr<GuardedThread> _downloaderEventLoopThread;
         config::ConfigFetcher _configFetcher;
 
         template <class T>
@@ -73,22 +95,16 @@ class FileDistributor : public config::IFetcherCallback<ZookeepersConfig>,
                    const FiledistributorConfig& fileDistributorConfig,
                    const FiledistributorrpcConfig& rpcConfig)
             :_zk(track(new ZKFacade(zooKeepersConfig.zookeeperserverlist))),
-             _model(track(new FileDistributionModelImpl(
-                                     fileDistributorConfig.hostname,
-                                     fileDistributorConfig.torrentport,
-                                     _zk))),
+             _model(track(new FileDistributionModelImpl(fileDistributorConfig.hostname, fileDistributorConfig.torrentport, _zk))),
              _tracker(track(new FileDistributorTrackerImpl(_model))),
-             _downloader(track(new FileDownloader(_tracker,
-                                                  fileDistributorConfig.hostname,
-                                                  fileDistributorConfig.torrentport,
-                                                  boost::filesystem::path(fileDistributorConfig.filedbpath)))),
+             _downloader(track(new FileDownloader(_tracker, fileDistributorConfig.hostname, fileDistributorConfig.torrentport, Path(fileDistributorConfig.filedbpath)))),
              _manager(track(new FileDownloaderManager(_downloader, _model))),
              _rpcHandler(track(new FileDistributorRPC(rpcConfig.connectionspec, _manager))),
              _stateServer(track(new StateServerImpl(fileDistributorConfig.stateport))),
              _downloaderEventLoopThread(),
              _configFetcher(configUri.getContext())
         {
-             _downloaderEventLoopThread = std::make_unique<std::thread>([downloader=_downloader] () { downloader->runEventLoop(); });
+            _downloaderEventLoopThread = std::make_unique<GuardedThread>(_downloader);
             _manager->start();
             _rpcHandler->start();
 
@@ -108,12 +124,7 @@ class FileDistributor : public config::IFetcherCallback<ZookeepersConfig>,
             //Do not waste time retrying zookeeper operations when going down.
             _zk->disableRetries();
 
-            _downloader->close();
-            _downloaderEventLoopThread->join();
-            if ( !_downloader->drained() ) {
-                LOG(error, "The filedownloader did not drain fully. We will just exit quickly and let a restart repair it for us.");
-                std::quick_exit(67);
-            }
+            _downloaderEventLoopThread.reset();
         }
 
     };
diff --git a/filedistribution/src/tests/rpc/mockfileprovider.h b/filedistribution/src/tests/rpc/mockfileprovider.h
index 745acc7196c..230cd0d0382 100644
--- a/filedistribution/src/tests/rpc/mockfileprovider.h
+++ b/filedistribution/src/tests/rpc/mockfileprovider.h
@@ -14,11 +14,11 @@ public:
 
     boost::barrier _queueForeverBarrier;
 
-    boost::optional<boost::filesystem::path> getPath(const std::string& fileReference) {
+    boost::optional<Path> getPath(const std::string& fileReference) {
         if (fileReference == "dd") {
-            return boost::filesystem::path("direct/result/path");
+            return Path("direct/result/path");
         } else {
-            return boost::optional<boost::filesystem::path>();
+            return boost::optional<Path>();
         }
     }
 
diff --git a/filedistribution/src/tests/zkfiledbmodel/test-zkfiledbmodel.cpp b/filedistribution/src/tests/zkfiledbmodel/test-zkfiledbmodel.cpp
index 6a3a87aac96..c102a235603 100644
--- a/filedistribution/src/tests/zkfiledbmodel/test-zkfiledbmodel.cpp
+++ b/filedistribution/src/tests/zkfiledbmodel/test-zkfiledbmodel.cpp
@@ -16,11 +16,8 @@
 
 using namespace filedistribution;
 
-typedef boost::filesystem::path Path;
-
 namespace {
 
-
 struct Fixture {
     ComponentsDeleter _componentsDeleter;
     std::shared_ptr<ZKFacade> zk;