fixed potential segfault

arangod/Dispatcher/ApplicationDispatcher.cpp

@@ -68,7 +68,6 @@ ApplicationDispatcher::ApplicationDispatcher()
     : ApplicationFeature("dispatcher"),
       _applicationScheduler(nullptr),
       _dispatcher(nullptr),
-      _dispatcherReporterTask(nullptr),
       _reportInterval(0.0),
       _nrStandardThreads(0),
       _nrAQLThreads(0) {}
@@ -236,10 +235,6 @@ void ApplicationDispatcher::stop() {
     return;
   }
 
-  if (_dispatcherReporterTask != nullptr) {
-    _dispatcherReporterTask = nullptr;
-  }
-
   if (_dispatcher != nullptr) {
     _dispatcher->shutdown();
     delete _dispatcher;
@@ -271,10 +266,10 @@ void ApplicationDispatcher::buildDispatcherReporter() {
   }
 
   if (0.0 < _reportInterval) {
-    _dispatcherReporterTask =
+    auto dispatcherReporterTask =
         new DispatcherReporterTask(_dispatcher, _reportInterval);
 
-    _applicationScheduler->scheduler()->registerTask(_dispatcherReporterTask);
+    _applicationScheduler->scheduler()->registerTask(dispatcherReporterTask);
   }
 }
 

arangod/Dispatcher/ApplicationDispatcher.h

@@ -164,12 +164,6 @@ class ApplicationDispatcher : virtual public ApplicationFeature {
 
   Dispatcher* _dispatcher;
 
-  //////////////////////////////////////////////////////////////////////////////
-  /// @brief reporting task
-  //////////////////////////////////////////////////////////////////////////////
-
-  Task* _dispatcherReporterTask;
-
   //////////////////////////////////////////////////////////////////////////////
   /// @brief interval for reports
   //////////////////////////////////////////////////////////////////////////////

arangod/HttpServer/HttpCommTask.cpp

@@ -585,7 +585,6 @@ void HttpCommTask::setupDone() {
   _setupDone.store(true, std::memory_order_relaxed);
 }
 
-
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief reads data from the socket
 ////////////////////////////////////////////////////////////////////////////////
@@ -966,6 +965,9 @@ bool HttpCommTask::setup(Scheduler* scheduler, EventLoop loop) {
 
   _scheduler = scheduler;
   _loop = loop;
+  
+  _server->registerChunkedTask(this);
+  setupDone();
 
   return true;
 }

arangod/HttpServer/HttpServer.cpp

@@ -171,7 +171,7 @@ void HttpServer::stopListening() {
 /// @brief registers a chunked task
 ////////////////////////////////////////////////////////////////////////////////
 
-void HttpServer::registerChunkedTask(HttpCommTask* task, ssize_t n) {
+void HttpServer::registerChunkedTask(HttpCommTask* task) {
   auto id = task->taskId();
   MUTEX_LOCKER(HttpCommTaskMapLock);
 
@@ -231,14 +231,7 @@ void HttpServer::handleConnected(TRI_socket_t s, ConnectionInfo const& info) {
 
   // registers the task and get the number of the scheduler thread
   ssize_t n;
-  int res = _scheduler->registerTask(task, &n);
-
-  // register the ChunkedTask in the same thread
-  if (res == TRI_ERROR_NO_ERROR) {
-    registerChunkedTask(task, n);
-  }
-
-  task->setupDone();
+  _scheduler->registerTask(task, &n);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -350,10 +343,14 @@ bool HttpServer::openEndpoint(Endpoint* endpoint) {
     return false;
   }
 
-  _scheduler->registerTask(task);
-  _listenTasks.emplace_back(task);
+  int res = _scheduler->registerTask(task);
+  
+  if (res == TRI_ERROR_NO_ERROR) {
+    _listenTasks.emplace_back(task);
+    return true;
+  }
 
-  return true;
+  return false;
 }
 
 ////////////////////////////////////////////////////////////////////////////////

arangod/HttpServer/HttpServer.h

@@ -146,7 +146,7 @@ class HttpServer : protected TaskManager {
   /// @brief registers a chunked task
   //////////////////////////////////////////////////////////////////////////////
 
-  void registerChunkedTask(HttpCommTask*, ssize_t);
+  void registerChunkedTask(HttpCommTask*);
 
   //////////////////////////////////////////////////////////////////////////////
   /// @brief unregisters a chunked task

arangod/Scheduler/ApplicationScheduler.cpp

@@ -321,18 +321,6 @@ void ApplicationScheduler::allowMultiScheduler(bool value) {
 
 Scheduler* ApplicationScheduler::scheduler() const { return _scheduler; }
 
-////////////////////////////////////////////////////////////////////////////////
-/// @brief installs a signal handler
-////////////////////////////////////////////////////////////////////////////////
-
-void ApplicationScheduler::installSignalHandler(SignalTask* task) {
-  if (_scheduler == nullptr) {
-    LOG_FATAL_AND_EXIT("no scheduler is known, cannot install signal handler");
-  }
-
-  _scheduler->registerTask(task);
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief returns the number of used threads
 ////////////////////////////////////////////////////////////////////////////////
@@ -563,8 +551,11 @@ void ApplicationScheduler::buildSchedulerReporter() {
   if (0.0 < _reportInterval) {
     Task* reporter = new SchedulerReporterTask(_scheduler, _reportInterval);
 
-    _scheduler->registerTask(reporter);
-    _tasks.emplace_back(reporter);
+    int res = _scheduler->registerTask(reporter);
+
+    if (res == TRI_ERROR_NO_ERROR) {
+      _tasks.emplace_back(reporter);
+    }
   }
 }
 
@@ -582,21 +573,30 @@ void ApplicationScheduler::buildControlCHandler() {
     // control C handler
     Task* controlC = new ControlCTask(_applicationServer);
 
-    _scheduler->registerTask(controlC);
-    _tasks.emplace_back(controlC);
+    int res = _scheduler->registerTask(controlC);
+
+    if (res == TRI_ERROR_NO_ERROR) {
+      _tasks.emplace_back(controlC);
+    }
   }
 
   // hangup handler
   Task* hangup = new HangupTask();
 
-  _scheduler->registerTask(hangup);
-  _tasks.emplace_back(hangup);
+  int res = _scheduler->registerTask(hangup);
+
+  if (res == TRI_ERROR_NO_ERROR) {
+    _tasks.emplace_back(hangup);
+  }
 
   // sigusr handler
   Task* sigusr = new Sigusr1Task(this);
 
-  _scheduler->registerTask(sigusr);
-  _tasks.emplace_back(sigusr);
+  res = _scheduler->registerTask(sigusr);
+
+  if (res == TRI_ERROR_NO_ERROR) {
+    _tasks.emplace_back(sigusr);
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////

arangod/Scheduler/ApplicationScheduler.h

@@ -68,12 +68,6 @@ class ApplicationScheduler : public ApplicationFeature {
 
   Scheduler* scheduler() const;
 
-  //////////////////////////////////////////////////////////////////////////////
-  /// @brief installs a signal handler
-  //////////////////////////////////////////////////////////////////////////////
-
-  void installSignalHandler(SignalTask*);
-
   //////////////////////////////////////////////////////////////////////////////
   /// @brief returns the number of used threads
   //////////////////////////////////////////////////////////////////////////////

arangod/Scheduler/Scheduler.cpp

@@ -338,14 +338,6 @@ int Scheduler::registerTask(Task* task, ssize_t* tn) {
   return registerTask(task, tn, -1);
 }
 
-////////////////////////////////////////////////////////////////////////////////
-/// @brief registers a new task with a pre-selected thread number
-////////////////////////////////////////////////////////////////////////////////
-
-int Scheduler::registerTaskInThread(Task* task, ssize_t tn) {
-  return registerTask(task, nullptr, tn);
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief unregisters a task
 ////////////////////////////////////////////////////////////////////////////////
@@ -448,11 +440,16 @@ Task* Scheduler::lookupTaskById(uint64_t taskId) {
 
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief registers a new task
+/// the caller must not use the task object afterwards, as it may be deleted
+/// by this function or a SchedulerThread
 ////////////////////////////////////////////////////////////////////////////////
 
 int Scheduler::registerTask(Task* task, ssize_t* got, ssize_t want) {
+  TRI_ASSERT(task != nullptr);
+
   if (task->isUserDefined() && task->id().empty()) {
     // user-defined task without id is invalid
+    deleteTask(task);
     return TRI_ERROR_TASK_INVALID_ID;
   }
 
@@ -466,16 +463,18 @@ int Scheduler::registerTask(Task* task, ssize_t* got, ssize_t want) {
     n = want;
 
     if (nrThreads <= n) {
+      deleteTask(task);
       return TRI_ERROR_INTERNAL;
     }
   }
 
-  {
+  try {
     MUTEX_LOCKER(schedulerLock);
 
     int res = checkInsertTask(task);
 
     if (res != TRI_ERROR_NO_ERROR) {
+      deleteTask(task);
       return res;
     }
 
@@ -490,12 +489,18 @@ int Scheduler::registerTask(Task* task, ssize_t* got, ssize_t want) {
     task2thread[task] = thread;
     taskRegistered[task->taskId()] = task;
   }
+  catch (...) {
+    destroyTask(task);
+    throw;
+  }
 
   if (nullptr != got) {
     *got = static_cast<ssize_t>(n);
   }
 
   if (!thread->registerTask(this, task)) {
+    // no need to delete the task here, as SchedulerThread::registerTask
+    // takes over the ownership
     return TRI_ERROR_INTERNAL;
   }
 

arangod/Scheduler/Scheduler.h

@@ -166,12 +166,6 @@ class Scheduler : private TaskManager {
 
   int registerTask(Task*, ssize_t* n);
 
-  //////////////////////////////////////////////////////////////////////////////
-  /// @brief registers a new task with a pre-selected thread number
-  //////////////////////////////////////////////////////////////////////////////
-
-  int registerTaskInThread(Task* task, ssize_t tn);
-
   //////////////////////////////////////////////////////////////////////////////
   /// @brief unregisters a task
   ///

arangod/Scheduler/SchedulerThread.cpp

@@ -104,6 +104,7 @@ bool SchedulerThread::registerTask(Scheduler* scheduler, Task* task) {
   // thread has already been stopped
   if (_stopped.load()) {
     // do nothing
+    deleteTask(task);
     return false;
   }
 

arangod/V8Server/v8-dispatcher.cpp

@@ -219,14 +219,6 @@ static void JS_RegisterTask(const v8::FunctionCallbackInfo<v8::Value>& args) {
   std::unique_ptr<TRI_json_t> json(task->toJson());
 
   if (json == nullptr) {
-    if (period > 0.0) {
-      V8PeriodicTask* t = dynamic_cast<V8PeriodicTask*>(task);
-      delete t;
-    } else {
-      V8TimerTask* t = dynamic_cast<V8TimerTask*>(task);
-      delete t;
-    }
-
     TRI_V8_THROW_EXCEPTION_MEMORY();
   }