Run foreground tasks on the isolate thread they should run on, cancel tasks scheduled in the future when the isolate is destructed

Author: Martijn Otto

Makefile

@@ -148,5 +148,5 @@ install:
 						${CP} ${INI} ${INI_DIR}
 
 clean:
-						${RM} ${EXTENSION} ${OBJECTS}
+						${RM} ${EXTENSION} ${OBJECTS} ${DEPENDENCIES}
 

isolate.cpp

@@ -21,7 +21,6 @@
 #include "isolate.h"
 #include <cstring>
 #include <cstdlib>
-#include <iostream>
 
 /**
  *  Start namespace
@@ -69,6 +68,10 @@ Isolate::Isolate()
     // create the actual isolate
     _isolate = v8::Isolate::New(params);
 
+    // associate ourselves with this isolate
+    // so that we can find it back from the pointer
+    _isolate->SetData(0, this);
+
     // and enter it
     _isolate->Enter();
 }
@@ -78,13 +81,85 @@ Isolate::Isolate()
  */
 Isolate::~Isolate()
 {
+    // run all tasks still awaiting executing
+    runTasks();
+
+    /**
+     *  the tasks that are still there are scheduled
+     *  somewhere in the future, we just delete these
+     *  - this seems like a strange thing to do, but
+     *  executing them now seems to throw v8 off guard
+     *  and results in either a deadlock or some weird
+     *  error message about garbage collection running
+     *  in some old "space" or something equally weird
+     *  and confusing.
+     */
+    _tasks.clear();
+
     // leave the isolate scope
     _isolate->Exit();
 
     // clean it up
     _isolate->Dispose();
 }
 
+/**
+ *  Perform all waiting tasks for this isolate
+ */
+void Isolate::runTasks()
+{
+    // no tasks? then we're done fast!
+    if (_tasks.empty()) return;
+
+    // determine the current time
+    auto now = std::chrono::system_clock::now();
+
+    // loop over all the tasks
+    for (auto iter = _tasks.begin(); iter != _tasks.end(); ++iter)
+    {
+        // is the execution time still in the future?
+        if (now < iter->first)
+        {
+            // first task? then don't remove anything
+            if (iter == _tasks.begin()) return;
+
+            // remove from the beginning up until the task
+            // since we already moved past the last task we
+            // need to go back one so we don't remove a task
+            // we have not actually executed yet
+            _tasks.erase(_tasks.begin(), --iter);
+
+            // tasks executed and removed
+            return;
+        }
+
+        // execute the task
+        iter->second->Run();
+    }
+
+    // we ran through all the tasks and executed all of them
+    _tasks.clear();
+}
+
+/**
+ *  Schedule a task to be executed
+ *
+ *  @param  isolate The isolate to execute the task under
+ *  @param  task    The task to execute
+ *  @param  delay   Number of seconds to wait before executing
+ */
+void Isolate::scheduleTask(v8::Isolate *isolate, v8::Task *task, double delay)
+{
+    // first retrieve the isolate to schedule it under
+    auto *real = static_cast<Isolate*>(isolate->GetData(0));
+
+    // determine the time at which the task should be executed
+    auto expire = std::chrono::system_clock::now() + std::chrono::microseconds{ static_cast<int64_t>(delay * 1000000) };
+
+    // schedule the task to be executed
+    real->_tasks.emplace(std::make_pair(expire, std::unique_ptr<v8::Task>{ task }));
+}
+
 /**
  *  Get the isolate for this thread
  *
@@ -95,6 +170,9 @@ v8::Isolate *Isolate::get()
     // do we still have to create the isolate?
     if (!isolate) isolate.reset(new Isolate);
 
+    // execute tasks for this isolate
+    isolate->runTasks();
+
     // return the isolate
     return *isolate;
 }

isolate.h

@@ -22,7 +22,10 @@
 /**
  *  Dependencies
  */
+#include <v8-platform.h>
+#include <chrono>
 #include <v8.h>
+#include <map>
 
 /**
  *  Start namespace
@@ -40,6 +43,17 @@ class Isolate final
      *  @var    v8::Isolate*
      */
     v8::Isolate *_isolate;
+
+    /**
+     *  List of tasks to execute
+     *  @var    std::multimap<std::chrono::system_clock::time_point, std::unique_ptr<v8::Task>>
+     */
+    std::multimap<std::chrono::system_clock::time_point, std::unique_ptr<v8::Task>> _tasks;
+
+    /**
+     *  Perform all waiting tasks for this isolate
+     */
+    void runTasks();
 public:
     /**
      *  Constructor
@@ -51,6 +65,15 @@ class Isolate final
      */
     ~Isolate();
 
+    /**
+     *  Schedule a task to be executed
+     *
+     *  @param  isolate The isolate to schedule the task under
+     *  @param  task    The task to execute
+     *  @param  delay   Number of seconds to wait before executing
+     */
+    static void scheduleTask(v8::Isolate *isolate, v8::Task *task, double delay);
+
     /**
      *  Get the isolate for this thread
      *

platform.cpp

@@ -13,6 +13,7 @@
  */
 #include <v8.h>
 #include "platform.h"
+#include "isolate.h"
 #include <atomic>
 #include <time.h>
 #include <mutex>
@@ -185,7 +186,7 @@ void Platform::shutdown()
      *  all PHP engines to be destructed, but we actually do
      *  stay loaded in memory. So we have to make sure that
      *  this variable is correctly set to a nullptr, so that
-     *  the get() method creates a new instance.
+     *  the create() method creates a new instance.
      */
 
     // retrieve the current platform
@@ -314,10 +315,8 @@ void Platform::CallOnForegroundThread(v8::Isolate *isolate, v8::Task *task)
  */
 void Platform::CallDelayedOnForegroundThread(v8::Isolate *isolate, v8::Task *task, double delay_in_seconds)
 {
-    // we simply call the CallOnBackgroundThread method which will queue our task, but before that
-    // we turn it into a DelayedTask. The ExpectedRuntime here doesn't matter as our implementation
-    // of CallOnBackgroundThread doesn't do anything with it anyway
-    CallOnBackgroundThread(new DelayedTask(task, delay_in_seconds), ExpectedRuntime::kShortRunningTask);
+    // schedule this task to be executed in the isolate
+    Isolate::scheduleTask(isolate, task, delay_in_seconds);
 }
 
 /**