From 62918c2fd0f227db10e86deb5c9dee636a17e304 Mon Sep 17 00:00:00 2001 From: Martijn Otto Date: Fri, 15 Apr 2016 13:48:05 +0200 Subject: [PATCH] Run foreground tasks on the isolate thread they should run on, cancel tasks scheduled in the future when the isolate is destructed --- Makefile | 2 +- isolate.cpp | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++- isolate.h | 23 +++++++++++++++ platform.cpp | 9 +++--- 4 files changed, 107 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index db7e45a..068d1d7 100644 --- a/Makefile +++ b/Makefile @@ -148,5 +148,5 @@ install: ${CP} ${INI} ${INI_DIR} clean: - ${RM} ${EXTENSION} ${OBJECTS} + ${RM} ${EXTENSION} ${OBJECTS} ${DEPENDENCIES} diff --git a/isolate.cpp b/isolate.cpp index 5d45b7c..1e6856b 100644 --- a/isolate.cpp +++ b/isolate.cpp @@ -21,7 +21,6 @@ #include "isolate.h" #include #include -#include /** * 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,6 +81,21 @@ 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(); @@ -85,6 +103,63 @@ Isolate::~Isolate() _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->GetData(0)); + + // determine the time at which the task should be executed + auto expire = std::chrono::system_clock::now() + std::chrono::microseconds{ static_cast(delay * 1000000) }; + + // schedule the task to be executed + real->_tasks.emplace(std::make_pair(expire, std::unique_ptr{ 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; } diff --git a/isolate.h b/isolate.h index 0046973..7c217a1 100644 --- a/isolate.h +++ b/isolate.h @@ -22,7 +22,10 @@ /** * Dependencies */ +#include +#include #include +#include /** * Start namespace @@ -40,6 +43,17 @@ class Isolate final * @var v8::Isolate* */ v8::Isolate *_isolate; + + /** + * List of tasks to execute + * @var std::multimap> + */ + std::multimap> _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 * diff --git a/platform.cpp b/platform.cpp index b0193d9..bb8153d 100644 --- a/platform.cpp +++ b/platform.cpp @@ -13,6 +13,7 @@ */ #include #include "platform.h" +#include "isolate.h" #include #include #include @@ -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); } /**