Skip to content

Commit

Permalink
Manually track external references, as v8 does not appear capable of …
Browse files Browse the repository at this point in the history
…freeing externals in a timely manner - not even if you shutdown the entire v8 platform
  • Loading branch information
Martijn Otto committed Nov 16, 2015
1 parent d7a5ad1 commit dcfeb8a
Show file tree
Hide file tree
Showing 7 changed files with 245 additions and 109 deletions.
18 changes: 9 additions & 9 deletions array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace JS {
* @param array The array to count
* @return the number of numeric, sequential keys
*/
static uint32_t count(const Php::Array &array)
static uint32_t findMax(const Php::Value &array)
{
// the variable to store count
int64_t result = 0;
Expand Down Expand Up @@ -60,7 +60,7 @@ static void enumerator(const v8::PropertyCallbackInfo<v8::Array> &info)
{
// create a local handle, so properties "fall out of scope" and retrieve the original object
v8::HandleScope scope(Isolate::get());
Handle<Php::Array> handle(info.Data());
Handle handle(info.Data());

// create a new array to store all the properties
v8::Local<v8::Array> properties(v8::Array::New(Isolate::get()));
Expand Down Expand Up @@ -90,7 +90,7 @@ static void getter(uint32_t index, const v8::PropertyCallbackInfo<v8::Value> &in
{
// create a local handle, so properties "fall out of scope" and retrieve the original object
v8::HandleScope scope(Isolate::get());
Handle<Php::Array> handle(info.Data());
Handle handle(info.Data());

// check if we have an item at the requested offset
if (handle->contains(index))
Expand All @@ -117,7 +117,7 @@ static void getter(v8::Local<v8::String> property, const v8::PropertyCallbackInf
v8::HandleScope scope(Isolate::get());

// retrieve handle to the original object and the property name
Handle<Php::Array> handle(info.Data());
Handle handle(info.Data());
v8::String::Utf8Value name(property);

// check if the property exists
Expand All @@ -129,7 +129,7 @@ static void getter(v8::Local<v8::String> property, const v8::PropertyCallbackInf
else if (std::strcmp(*name, "length") == 0)
{
// return the count from this array
info.GetReturnValue().Set(count(*handle));
info.GetReturnValue().Set(findMax(*handle));
}
else
{
Expand All @@ -148,7 +148,7 @@ static void getter(v8::Local<v8::String> property, const v8::PropertyCallbackInf
static void setter(uint32_t index, v8::Local<v8::Value> input, const v8::PropertyCallbackInfo<v8::Value>& info)
{
// retrieve handle to the original object
Handle<Php::Array> handle(info.Data());
Handle handle(info.Data());

// store the property inside the array
handle->set(index, value(input));
Expand All @@ -164,7 +164,7 @@ static void setter(uint32_t index, v8::Local<v8::Value> input, const v8::Propert
static void setter(v8::Local<v8::String> property, v8::Local<v8::Value> input, const v8::PropertyCallbackInfo<v8::Value>& info)
{
// retrieve handle to the original object and convert the requested property
Handle<Php::Array> handle(info.Data());
Handle handle(info.Data());
v8::String::Utf8Value name(property);

// store the property inside the array
Expand All @@ -180,8 +180,8 @@ Array::Array(Php::Array array) :
_template(v8::ObjectTemplate::New())
{
// register the property handlers
_template->SetNamedPropertyHandler(getter, setter, nullptr, nullptr, enumerator, Handle<Php::Array>(array));
_template->SetIndexedPropertyHandler(getter, setter, nullptr, nullptr, nullptr, Handle<Php::Array>(array));
_template->SetNamedPropertyHandler(getter, setter, nullptr, nullptr, enumerator, Handle(array));
_template->SetIndexedPropertyHandler(getter, setter, nullptr, nullptr, nullptr, Handle(array));
}

/**
Expand Down
51 changes: 51 additions & 0 deletions context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
/**
* Dependencies
*/
#include "external.h"
#include "context.h"
#include "isolate.h"
#include "value.h"
Expand All @@ -35,8 +36,58 @@ Context::Context()

// now create the context
_context = v8::Context::New(Isolate::get(), nullptr);

// store a link to ourselves
_context->SetAlignedPointerInEmbedderData(1, this);
}

/**
* Destructor
*/
Context::~Context()
{
// destroy all externals
for (auto *external : _externals) delete external;
}

/**
* Retrieve the currently active context
*
* @return The current context, or a nullptr
*/
Context *Context::current()
{
// retrieve the current context
auto context = Isolate::get()->GetEnteredContext();

// if no context is available we are out of options
if (context.IsEmpty()) return nullptr;

// retrieve the context from the handle
return reinterpret_cast<Context*>(context->GetAlignedPointerFromEmbedderData(1));
}

/**
* Track a new external object
*
* @var External*
*/
void Context::track(External *external)
{
// add to the list of tracked references
_externals.insert(external);
}

/**
* Unregister an external object
*
* @var external The external object we no longer to track
*/
void Context::untrack(External *external)
{
// remove from the list of tracked references
_externals.erase(external);
}

/**
* Assign a variable to the javascript context
Expand Down
37 changes: 37 additions & 0 deletions context.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@
*/
namespace JS {

/**
* Forward declarations
*/
class External;

/**
* Class definition
*/
Expand All @@ -35,6 +40,12 @@ class Context : public Php::Base
* @var Stack<V8::Context>
*/
Stack<v8::Context> _context;

/**
* List of external pointers to track
* @var std::set<External>
*/
std::set<External*> _externals;
public:
/**
* Constructor
Expand All @@ -55,6 +66,32 @@ class Context : public Php::Base
*/
Context(Context&&that) = default;

/**
* Destructor
*/
virtual ~Context();

/**
* Retrieve the currently active context
*
* @return The current context, or a nullptr
*/
static Context *current();

/**
* Track a new external object
*
* @var External*
*/
void track(External *external);

/**
* Unregister an external object
*
* @var external The external object we no longer to track
*/
void untrack(External *external);

/**
* Assign a variable to the javascript context
*
Expand Down
113 changes: 113 additions & 0 deletions external.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/**
* external.h
*
* Class that tracks a pointer external to the V8
* engine that we should clean up if V8 somehow
* forgets (which happens a lot).
*
* @copyright 2015 Copernica B.V.
*/

/**
* Include guard
*/
#pragma once

/**
* Dependencies
*/
#include <v8.h>
#include "context.h"

/**
* Start namespace
*/
namespace JS {

/**
* Object holding a reference external to V8
*/
class External
{
private:
/**
* The allocated object
* @var Php::Value
*/
Php::Value _object;

/**
* The persistent handle
* @var v8::Persistent<v8::Value>
*/
v8::Persistent<v8::Value> _persistent;

/**
* The destructor callback to clean up the object
*
* @param data callback data
*/
static void destructor(const v8::WeakCallbackData<v8::Value, External> &data)
{
// stop tracking the external reference
Context::current()->untrack(data.GetParameter());

// delete the object
delete data.GetParameter();
}
public:
/**
* Constructor
*
* @param object The object to keep in memory
*/
External(Php::Value &&object) :
_object(std::move(object))
// persistent will be initialized later
{
// start tracking the reference
Context::current()->track(this);
}

/**
* Destructor
*/
~External()
{
/**
* Reset the persistent handle
*
* One would assume the fact that the persistent is destructed
* would be enough indication to v8 that the handle is now garbage,
* but alas, if we don't call this v8 will trip over and assume
* that the object is still alive and then complain about it.
*/
_persistent.Reset();
}

/**
* Initialize the persistent handle
*/
void initialize(const v8::Local<v8::External> &handle)
{
// create the persistent handle and make it weak
_persistent.Reset(Isolate::get(), handle);
_persistent.SetWeak<External>(this, &destructor);
}

/**
* Get pointer to the underlying object
*
* @return The underlying value
*/
Php::Value *get()
{
// return the pointer
return &_object;
}
};

/**
* End namespace
*/
}
Loading

0 comments on commit dcfeb8a

Please sign in to comment.