Skip to content

Commit dcfeb8a

Browse files
author
Martijn Otto
committed
Manually track external references, as v8 does not appear capable of freeing externals in a timely manner - not even if you shutdown the entire v8 platform
1 parent d7a5ad1 commit dcfeb8a

File tree

7 files changed

+245
-109
lines changed

7 files changed

+245
-109
lines changed

array.cpp

+9-9
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ namespace JS {
2828
* @param array The array to count
2929
* @return the number of numeric, sequential keys
3030
*/
31-
static uint32_t count(const Php::Array &array)
31+
static uint32_t findMax(const Php::Value &array)
3232
{
3333
// the variable to store count
3434
int64_t result = 0;
@@ -60,7 +60,7 @@ static void enumerator(const v8::PropertyCallbackInfo<v8::Array> &info)
6060
{
6161
// create a local handle, so properties "fall out of scope" and retrieve the original object
6262
v8::HandleScope scope(Isolate::get());
63-
Handle<Php::Array> handle(info.Data());
63+
Handle handle(info.Data());
6464

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

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

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

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

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

170170
// store the property inside the array
@@ -180,8 +180,8 @@ Array::Array(Php::Array array) :
180180
_template(v8::ObjectTemplate::New())
181181
{
182182
// register the property handlers
183-
_template->SetNamedPropertyHandler(getter, setter, nullptr, nullptr, enumerator, Handle<Php::Array>(array));
184-
_template->SetIndexedPropertyHandler(getter, setter, nullptr, nullptr, nullptr, Handle<Php::Array>(array));
183+
_template->SetNamedPropertyHandler(getter, setter, nullptr, nullptr, enumerator, Handle(array));
184+
_template->SetIndexedPropertyHandler(getter, setter, nullptr, nullptr, nullptr, Handle(array));
185185
}
186186

187187
/**

context.cpp

+51
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
/**
1111
* Dependencies
1212
*/
13+
#include "external.h"
1314
#include "context.h"
1415
#include "isolate.h"
1516
#include "value.h"
@@ -35,8 +36,58 @@ Context::Context()
3536

3637
// now create the context
3738
_context = v8::Context::New(Isolate::get(), nullptr);
39+
40+
// store a link to ourselves
41+
_context->SetAlignedPointerInEmbedderData(1, this);
3842
}
3943

44+
/**
45+
* Destructor
46+
*/
47+
Context::~Context()
48+
{
49+
// destroy all externals
50+
for (auto *external : _externals) delete external;
51+
}
52+
53+
/**
54+
* Retrieve the currently active context
55+
*
56+
* @return The current context, or a nullptr
57+
*/
58+
Context *Context::current()
59+
{
60+
// retrieve the current context
61+
auto context = Isolate::get()->GetEnteredContext();
62+
63+
// if no context is available we are out of options
64+
if (context.IsEmpty()) return nullptr;
65+
66+
// retrieve the context from the handle
67+
return reinterpret_cast<Context*>(context->GetAlignedPointerFromEmbedderData(1));
68+
}
69+
70+
/**
71+
* Track a new external object
72+
*
73+
* @var External*
74+
*/
75+
void Context::track(External *external)
76+
{
77+
// add to the list of tracked references
78+
_externals.insert(external);
79+
}
80+
81+
/**
82+
* Unregister an external object
83+
*
84+
* @var external The external object we no longer to track
85+
*/
86+
void Context::untrack(External *external)
87+
{
88+
// remove from the list of tracked references
89+
_externals.erase(external);
90+
}
4091

4192
/**
4293
* Assign a variable to the javascript context

context.h

+37
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@
2424
*/
2525
namespace JS {
2626

27+
/**
28+
* Forward declarations
29+
*/
30+
class External;
31+
2732
/**
2833
* Class definition
2934
*/
@@ -35,6 +40,12 @@ class Context : public Php::Base
3540
* @var Stack<V8::Context>
3641
*/
3742
Stack<v8::Context> _context;
43+
44+
/**
45+
* List of external pointers to track
46+
* @var std::set<External>
47+
*/
48+
std::set<External*> _externals;
3849
public:
3950
/**
4051
* Constructor
@@ -55,6 +66,32 @@ class Context : public Php::Base
5566
*/
5667
Context(Context&&that) = default;
5768

69+
/**
70+
* Destructor
71+
*/
72+
virtual ~Context();
73+
74+
/**
75+
* Retrieve the currently active context
76+
*
77+
* @return The current context, or a nullptr
78+
*/
79+
static Context *current();
80+
81+
/**
82+
* Track a new external object
83+
*
84+
* @var External*
85+
*/
86+
void track(External *external);
87+
88+
/**
89+
* Unregister an external object
90+
*
91+
* @var external The external object we no longer to track
92+
*/
93+
void untrack(External *external);
94+
5895
/**
5996
* Assign a variable to the javascript context
6097
*

external.h

+113
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
/**
2+
* external.h
3+
*
4+
* Class that tracks a pointer external to the V8
5+
* engine that we should clean up if V8 somehow
6+
* forgets (which happens a lot).
7+
*
8+
* @copyright 2015 Copernica B.V.
9+
*/
10+
11+
/**
12+
* Include guard
13+
*/
14+
#pragma once
15+
16+
/**
17+
* Dependencies
18+
*/
19+
#include <v8.h>
20+
#include "context.h"
21+
22+
/**
23+
* Start namespace
24+
*/
25+
namespace JS {
26+
27+
/**
28+
* Object holding a reference external to V8
29+
*/
30+
class External
31+
{
32+
private:
33+
/**
34+
* The allocated object
35+
* @var Php::Value
36+
*/
37+
Php::Value _object;
38+
39+
/**
40+
* The persistent handle
41+
* @var v8::Persistent<v8::Value>
42+
*/
43+
v8::Persistent<v8::Value> _persistent;
44+
45+
/**
46+
* The destructor callback to clean up the object
47+
*
48+
* @param data callback data
49+
*/
50+
static void destructor(const v8::WeakCallbackData<v8::Value, External> &data)
51+
{
52+
// stop tracking the external reference
53+
Context::current()->untrack(data.GetParameter());
54+
55+
// delete the object
56+
delete data.GetParameter();
57+
}
58+
public:
59+
/**
60+
* Constructor
61+
*
62+
* @param object The object to keep in memory
63+
*/
64+
External(Php::Value &&object) :
65+
_object(std::move(object))
66+
// persistent will be initialized later
67+
{
68+
// start tracking the reference
69+
Context::current()->track(this);
70+
}
71+
72+
/**
73+
* Destructor
74+
*/
75+
~External()
76+
{
77+
/**
78+
* Reset the persistent handle
79+
*
80+
* One would assume the fact that the persistent is destructed
81+
* would be enough indication to v8 that the handle is now garbage,
82+
* but alas, if we don't call this v8 will trip over and assume
83+
* that the object is still alive and then complain about it.
84+
*/
85+
_persistent.Reset();
86+
}
87+
88+
/**
89+
* Initialize the persistent handle
90+
*/
91+
void initialize(const v8::Local<v8::External> &handle)
92+
{
93+
// create the persistent handle and make it weak
94+
_persistent.Reset(Isolate::get(), handle);
95+
_persistent.SetWeak<External>(this, &destructor);
96+
}
97+
98+
/**
99+
* Get pointer to the underlying object
100+
*
101+
* @return The underlying value
102+
*/
103+
Php::Value *get()
104+
{
105+
// return the pointer
106+
return &_object;
107+
}
108+
};
109+
110+
/**
111+
* End namespace
112+
*/
113+
}

0 commit comments

Comments
 (0)