C++ lifetime management via shared_ptr and

[loude...@gmail.com]

Like a good C++ programmer, I'm trying to use shared_ptrs to manage the lifetime of my C++ objects.  I would expect this code below in testv8() to invoke my ~Point destructor, but it's never called!  Can anyone tell me why?  

Also, I have some additional questions inline.  Thanks for your help!  

-Jim Acquavella, Adobe Systems Inc.

//Sample class mapped to v8

class Point 

{ 

public: 

//constructor

Point(int x, int y):x_(x),y_(y){}

virtual ~Point()

{

assert(false); // Never called!

}

//variables

int x_, y_; 

};

v8::Handle<v8::Value> GetPointX(v8::Local<v8::String> property,

const v8::AccessorInfo &info) {

using namespace v8;

Local<Object> self = info.Holder();

Local<External> wrap = Local<External>::Cast(self->GetInternalField(0));

void* ptr = wrap->Value();

int value = static_cast<Point*>(ptr)->x_;

return Integer::New(value);

}

void SetPointX(v8::Local<v8::String> property, v8::Local<v8::Value> value,

const v8::AccessorInfo& info) {

using namespace v8;

Local<Object> self = info.Holder();

Local<External> wrap = Local<External>::Cast(self->GetInternalField(0));

void* ptr = wrap->Value();

static_cast<Point*>(ptr)->x_ = value->Int32Value();

}

void testv8() 

{

{

v8::V8::Initialize();  // Initialize v8.  (Is this needed??  If so, why isn't it mentioned here -> https://developers.google.com/v8/embed)

// Get the default Isolate created at startup.

v8::Isolate* isolateP = v8::Isolate::GetCurrent();

// Create a stack-allocated handle scope.

v8::HandleScope handle_scope(isolateP);

// Create a template for the global object where we set the

// built-in global functions.

v8::Handle<v8::ObjectTemplate> global = v8::ObjectTemplate::New();

// Create a new context.

std::shared_ptr<v8::Persistent<v8::Context> > contextP(new v8::Persistent<v8::Context>(v8::Context::New(NULL, global)));

std::shared_ptr<v8::Context::Scope> contextScopeP(new v8::Context::Scope(*contextP));

{

using namespace v8;

Handle<FunctionTemplate> point_templ = FunctionTemplate::New();

point_templ->SetClassName(String::New("Point"));

Handle<ObjectTemplate> point_proto = point_templ->PrototypeTemplate();

Handle<ObjectTemplate> point_inst = point_templ->InstanceTemplate();

point_inst->SetInternalFieldCount(1);

point_inst->SetAccessor(String::New("x"), GetPointX, SetPointX);

Handle<Function> point_ctor = point_templ->GetFunction();

std::shared_ptr<Point> ptP(new Point(0, 0));

v8::Persistent<v8::Object> obj = wrapSharedPtr(point_ctor, ptP);

assert(obj.IsWeak(isolateP));

(*contextP)->Global()->Set(String::New("point"), obj);

obj.Dispose(isolateP);

Handle<String> source = String::New("point.x = 3;");

Handle<Script> script = Script::Compile(source);

Handle<Value> result = script->Run();

source = String::New("point.x;");

script = Script::Compile(source);

result = script->Run();

std::int32_t resultValue32 = result->Int32Value();

assert(3 == resultValue32);

}

// Dispose the persistent context.

contextP->Dispose(isolateP);

contextP->Clear();

contextScopeP.reset();

contextP.reset();

}

v8::V8::LowMemoryNotification();  // I saw this mentioned as a way to force GC, but it doesn't help.  Even so, I would think v8::V8::Dispose would clear all garbage.

v8::V8::Dispose();   // Initialize v8.  (Is this needed??  If so, why isn't it mentioned here -> https://developers.google.com/v8/embed)

}

[Stephan Beal]

On Fri, May 24, 2013 at 1:53 AM, <loude...@gmail.com> wrote:

virtual ~Point()

{

assert(false); // Never called!

}

v8 does not guaranty that it will _ever_ call your destructors. It does _not_ clean up automatically when it shuts down. This of course makes life difficult for types which require proper closing or close ordering (e.g., db statements and their drivers), but v8 is really centered on types which can be "thrown away" without running a destructor.

 

 

v8::V8::Initialize();  // Initialize v8.  (Is this needed??  If so, why isn't it mentioned here -> https://developers.google.com/v8/embed)

i've been using v8 since 2008 and never seen/used it.

 

v8::V8::LowMemoryNotification();  // I saw this mentioned as a way to force GC, but it doesn't help.  Even so, I would think v8::V8::Dispose would clear all garbage.

v8::V8::Dispose();   // Initialize v8.  (Is this needed??  If so, why isn't it mentioned here -> https://developers.google.com/v8/embed)

Whether or not it is needed, i don't know (i don't remember consistently using it). It won't, however, guaranty to run any cleanup of your bound types. In order for you to start seeing your dtor running, you need to create many thousands of them before v8 will start cleaning them up. In short-lived test apps the dtors are rarely run. In all of my bound native types i tend to bind a destroy() method which manually triggers the destruction process. Here's an example of how to do that:

https://code.google.com/p/v8-juice/source/browse/convert/include/cvv8/ClassCreator.hpp#980

-- 

----- stephan beal
http://wanderinghorse.net/home/stephan/

http://gplus.to/sgbeal

[Jim Acquavella]

Thanks Stephan!  I'm surprised and disappointed I can't guarentee my C++ objects will be destroyed.  Can anyone from Google comment on this?  There must be a way to force everything allocated to be released at shutdown, no?!

--
--
v8-users mailing list
v8-u...@googlegroups.com
http://groups.google.com/group/v8-users
---
You received this message because you are subscribed to a topic in the Google Groups "v8-users" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/v8-users/ta9wkdEY08o/unsubscribe?hl=en.
To unsubscribe from this group and all its topics, send an email to v8-users+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Stephan Beal]

On Fri, May 24, 2013 at 9:58 PM, Jim Acquavella <loude...@gmail.com> wrote:

Thanks Stephan!  I'm surprised and disappointed I can't guarentee my C++ objects will be destroyed.  Can anyone from Google comment on this?  There must be a way to force everything allocated to be released at shutdown, no?!

That horse has been beat to death here a few times already. No - there is no guaranty that v8 will ever call your dtors and (AFAIR) no 100% reliable way to force it to. To be safe, bind a destructor function to your native types.

[Sven Panne]

On Sat, May 25, 2013 at 4:36 AM, Stephan Beal <sgb...@googlemail.com> wrote:

On Fri, May 24, 2013 at 9:58 PM, Jim Acquavella <loude...@gmail.com> wrote:

Thanks Stephan!  I'm surprised and disappointed I can't guarentee my C++ objects will be destroyed.  Can anyone from Google comment on this?  There must be a way to force everything allocated to be released at shutdown, no?!

That horse has been beat to death here a few times already. No - there is no guaranty that v8 will ever call your dtors and (AFAIR) no 100% reliable way to force it to. To be safe, bind a destructor function to your native types.

Correct, in a nutshell: Finalizers are a very bad idea, use destructors.

   http://www.hpl.hp.com/techreports/2002/HPL-2002-335.pdf

   http://www.hpl.hp.com/personal/Hans_Boehm/popl03/slides.pdf

[Jim Acquavella]

I've updated the code and no longer explicitly calling obj.Dispose(isolateP), and my c++ object does get disposed when I call LowMemoryNotification before terminating v8.  The extra Dispose was clearing my near death callback.  This is good news and will greatly simplify my design and assure all c++ objects that share a javascript object are cleaned up.

> b.exe!beaker::console::Point::~Point()  Line 164 C++

  b.exe!beaker::console::Point::`scalar deleting destructor'()  + 0x2c bytes C++

  b.exe!std::tr1::_Ref_count<beaker::console::Point>::_Destroy()  Line 1107 + 0x34 bytes C++

  b.exe!std::tr1::_Ref_count_base::_Decref()  Line 1067 C++

  b.exe!std::tr1::_Ptr_base<beaker::console::Point>::_Decref()  Line 1291 C++

  b.exe!std::tr1::shared_ptr<beaker::console::Point>::~shared_ptr<beaker::console::Point>()  Line 1577 C++

  b.exe!std::tr1::shared_ptr<beaker::console::Point>::`scalar deleting destructor'()  + 0x2c bytes C++

  b.exe!beaker::console::detail::releaseSharedPtr<beaker::console::Point>(v8::Isolate * isolate, v8::Persistent<v8::Value> persistentObj, void * pData)  Line 42 + 0x2b bytes C++

  b.exe!v8::internal::GlobalHandles::Node::PostGarbageCollectionProcessing(v8::internal::Isolate * isolate, v8::internal::GlobalHandles * global_handles)  Line 288 C++

  b.exe!v8::internal::GlobalHandles::PostGarbageCollectionProcessing(v8::internal::GarbageCollector collector, v8::internal::GCTracer * tracer)  Line 667 + 0x25 bytes C++

  b.exe!v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector collector, v8::internal::GCTracer * tracer)  Line 1003 + 0x27 bytes C++

  b.exe!v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace space, v8::internal::GarbageCollector collector, const char * gc_reason, const char * collector_reason)  Line 661 + 0x19 bytes C++

  b.exe!v8::internal::Heap::CollectAllAvailableGarbage(const char * gc_reason)  Line 594 + 0x23 bytes C++

  b.exe!v8::V8::LowMemoryNotification()  Line 4968 C++

[Sven Panne]

On Wed, Jun 12, 2013 at 2:13 AM, Jim Acquavella <loude...@gmail.com> wrote:

I've updated the code and no longer explicitly calling obj.Dispose(isolateP), and my c++ object does get disposed when I call LowMemoryNotification before terminating v8.  The extra Dispose was clearing my near death callback.  This is good news and will greatly simplify my design and assure all c++ objects that share a javascript object are cleaned up. [...]

Warning notice: Even though it might work for you right now, I don't think that we give any guarantee that your callbacks are actually called in this scenario. Just to make sure that others don't use that "solution", too...

[Mike Moening]

Sven,

This problem keeps biting embedders over and over and over.

Even if Chrome does not need it...

The rest of the world needs a way to GUARANTEE that C++ backed JS objects get destroyed in a reliable and predicable manner.

There are many hacks people use to try to get weak callbacks to fire. This is just another attempt.

Can you or anyone propose a mechansim for fixing this?

I would be happy to attempt implementation if a decent proposal would come forward for:

1) Guaranteeing weak callbacks for all objects fire on shutdown of V8.   (necessary for proper memory leak testing)

2) A mechansim for telling V8 to collect garbage and fire weak callbacks on demand.

If we don't fix this right, everyone and their uncle will be using this "solution"...

We have no other choice.

Mike M.

[Jim Acquavella]

Thanks Mike M!  My solution above does seem to currently work, but the fact that Sven states this won't be guaranteed to work in the future is disappointing.  I expect the javascript objects in the heap that reference my C++ objects to be cleaned up when I quit my application.  In doing so, I want my C++ objects freed.  To me this seems like basic resource accounting that I expect v8 to support.  

Mike, here's my latest code that does free my C++ objects on termination:

{

v8::V8::Initialize();

// Get the default Isolate created at startup.

v8::Isolate* isolateP = v8::Isolate::GetCurrent();

// Create a stack-allocated handle scope.

v8::HandleScope handle_scope(isolateP);

// Create a template for the global object where we set the

// built-in global functions.

v8::Handle<v8::ObjectTemplate> global = v8::ObjectTemplate::New();

// Create a new context.

std::shared_ptr<v8::Persistent<v8::Context> > contextP(new v8::Persistent<v8::Context>(v8::Context::New(NULL, global)));

std::shared_ptr<v8::Context::Scope> contextScopeP(new v8::Context::Scope(*contextP));

{

using namespace v8;

Handle<FunctionTemplate> point_templ = FunctionTemplate::New();

point_templ->SetClassName(String::New("Point"));

Handle<ObjectTemplate> point_proto = point_templ->PrototypeTemplate();

point_proto->Set("method_a", FunctionTemplate::New(PointMethod_A));

point_proto->Set("method_b", FunctionTemplate::New(PointMethod_B));

Handle<ObjectTemplate> point_inst = point_templ->InstanceTemplate();

point_inst->SetInternalFieldCount(1);

point_inst->SetAccessor(String::New("x"), GetPointX, SetPointX);

Handle<Function> point_ctor = point_templ->GetFunction();

std::shared_ptr<Point> ptP(new Point(0, 0));

v8::Persistent<v8::Object> obj = wrapSharedPtr(point_ctor, ptP);

DVA_ASSERT(obj.IsWeak(isolateP));

(*contextP)->Global()->Set(String::New("point"), obj);

//obj.Dispose(isolateP);

Handle<String> source = String::New("point.x = 3;");

Handle<Script> script = Script::Compile(source);

Handle<Value> result = script->Run();

source = String::New("point.x;");

script = Script::Compile(source);

result = script->Run();

std::int32_t resultValue32 = result->Int32Value();

DVA_ASSERT(3 == resultValue32);

}

// Dispose the persistent context.

contextP->Dispose(isolateP);

contextP->Clear();

contextScopeP.reset();

contextP.reset();

}

v8::V8::LowMemoryNotification();

v8::V8::Dispose();

#ifdef BEAKER_DEBUG_V8_ALLOCATIONS

CheckV8AllocCountIsZero();

#endif

--

[Stephan Beal]

On Wed, Jun 12, 2013 at 6:48 PM, Jim Acquavella <loude...@gmail.com> wrote:

Thanks Mike M!  My solution above does seem to currently work, but the fact that Sven states this won't be guaranteed to work in the future is disappointing.  I expect the javascript objects in the heap that reference my C++ objects to be cleaned up when I quit my application.  In doing so, I want my C++ objects freed.  To me this seems like basic resource accounting that I expect v8 to support.  

Take it from one who's been there and back and there and back again... AFAIK it is not technically possible for v8 to safely call arbitrary client-side destructors in an arbitrary order. Supports we have two bound natives, the first one has a native level pointer to the second one, but the relationship is one way (the second doesn't know about the link). v8 comes along (hypothetically) and goes to call their dtors because it's philosophically the right thing to do. The order of their destruction will likely be arbitrary (based on their position in the gc'd memory block, or whatever), and let's assume it deletes the second one first. When it cleans up the first value, that value now holds a stale pointer to a destroyed value. Blammo. Post-main() segfault.

That same problem applies to non-deterministic gc in general, not just at v8 shutdown, and is one of the reasons one always needs to (==should) add a destroy() (or similar) method to bound natives when they have strict lifetime requirements. My canonical example is the DB driver and the statements it prepare()s, where the statement objects _must_ be freed before their owning DB is.

:/

[Mike Moening]

Your specific problem is easily solved with ref-counted objects on the C++ side.  I've done exactly this with your same use case. It works fine.

Please lets move the problem forward in a positive manner.

This is a problem that absolutely can and NEEDS to be solved.

[Stephan Beal]

On Wed, Jun 12, 2013 at 7:58 PM, Mike Moening <mike.m...@gmail.com> wrote:

Your specific problem is easily solved with ref-counted objects on the C++ side.  I've done exactly this with your same use case. It works fine.

Thanks for that tip :).

 

Please lets move the problem forward in a positive manner.

This is a problem that absolutely can and NEEDS to be solved.

But it's not going to be at the v8 level. A few years back one of the v8 devs said (in a post on this list, but i have no link handy) that v8 does not gc at shutdown because (i'm paraphrasing), "it negatively impacts Chrome's shutdown time." i think most of us will agree that shutting down cleanly is better than shutting down quickly, but JS was not really intended (it was pointed out in that thread or a similar one) to be used with types which require proper destructor calls to ensure proper behaviour of the system.

So, that's a "positive" answer in the sense that i'm positive v8 will never guaranty such a feature. v8's only _real_ concern, as far as directly adapting to customer needs, is Chromium and friends. (That's based off of my own observations over my years on this list, and not a sentiment expressed directly from anyone working on v8 (more implied by various answers they've provided).)

[Mike Moening]

"But it's not going to be at the v8 level." A few years back one of the v8 devs said (in a post on this list, but i have no link handy) that v8 does not gc at shutdown because (i'm paraphrasing), "it negatively impacts Chrome's shutdown time." i think most of us will agree that shutting down cleanly is better than shutting down quickly, but JS was not really intended (it was pointed out in that thread or a similar one) to be used with types which require proper destructor calls to ensure proper behaviour of the system.

So, that's a "positive" answer in the sense that i'm positive v8 will never guaranty such a feature. v8's only _real_ concern, as far as directly adapting to customer needs, is Chromium and friends. (That's based off of my own observations over my years on this list, and not a sentiment expressed directly from anyone working on v8 (more implied by various answers they've provided).)

That's not positive at all!  The past doesn't matter.  Nor does something that somebody said x months ago.  Please spare us from meaningless speculation.

Software evolves and changes all the time. (threading support via Isolates are an example of this)

Passing an optional boolean to indicate that full GC at shutdown is desired is very easy to add.

[Jim Acquavella]

As it stands now in my test code, it is optional because I'm calling v8::V8::LowMemoryNotification(); before v8::V8::Dispose();  If you want fast shutdown, don't call either!  Win, Win!

[Sven Panne]

Before we continue this useless discussion any further: Has anybody actually *read* Boehm's paper/slides?

[Jim Acquavella]

So, myth #5 in the slides seems like the thing that would most likely cause v8 to not know whether objects are marked as unreachable.  If I have a context, and a handle scope, that contains objects that reference each other, can't you determine the objects are unreachable if their parent context and handle scope are destroyed?

On Wed, Jun 12, 2013 at 11:43 PM, Sven Panne <sven...@chromium.org> wrote:

Before we continue this useless discussion any further: Has anybody actually *read* Boehm's paper/slides?

--

[Douglas Cox]

Has this issue been addressed at all? We're running into it now and it seems quite frustrating that there isn't just a parameter to properly clean up when uninitializing v8.  Even if it's just a debug only setting to help us spot leaks/bad code.  

Specifically if something has been placed in the Globals object.  We at least seem to be getting weak callbacks with IdleNotification() which seems like a hack instead of just calling V8::Dispose( PleaseCallWeakCallbacksNow );  I don't even see how this needs to do a GC.  It just needs to run through looking for weak object's that have a callback and call it...

[Ben Noordhuis]

You can walk live persistent handles with
v8::Isolate::VisitHandlesWithClassIds() if you give them a class id
with v8::Persistent<T>::SetWrapperClassId().

[Martin McDonough]

>"it negatively impacts Chrome's shutdown time."

This. This is something I had to learn time and again about V8. You are not working on Chrome, so the API will not be modified for you.