Skip to content

Commit

Permalink
Run foreground tasks on the isolate thread they should run on, cancel…
Browse files Browse the repository at this point in the history
… tasks scheduled in the future when the isolate is destructed
  • Loading branch information
Martijn Otto committed Apr 15, 2016
1 parent 09c9929 commit 62918c2
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 7 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -148,5 +148,5 @@ install:
${CP} ${INI} ${INI_DIR}

clean:
${RM} ${EXTENSION} ${OBJECTS}
${RM} ${EXTENSION} ${OBJECTS} ${DEPENDENCIES}

80 changes: 79 additions & 1 deletion isolate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include "isolate.h"
#include <cstring>
#include <cstdlib>
#include <iostream>

/**
* Start namespace
Expand Down Expand Up @@ -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();
}
Expand All @@ -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
*
Expand All @@ -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;
}
Expand Down
23 changes: 23 additions & 0 deletions isolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@
/**
* Dependencies
*/
#include <v8-platform.h>
#include <chrono>
#include <v8.h>
#include <map>

/**
* Start namespace
Expand All @@ -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
Expand All @@ -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
*
Expand Down
9 changes: 4 additions & 5 deletions platform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/
#include <v8.h>
#include "platform.h"
#include "isolate.h"
#include <atomic>
#include <time.h>
#include <mutex>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

/**
Expand Down

0 comments on commit 62918c2

Please sign in to comment.