looking for assistance with WeakPointer-based object destruction

[Stephan Beal]

Hi, all!

After having bound hundreds of C-like functions i'm ready to bind my
first class in v8. i've got it working so far except that my Weak
Pointer callback isn't being triggered, and i have been unable to
figure out why. i'm pasting the code here - it's a bit long, but i
would really appreciate any insights on this, as i can't do ANY class-
level wrapping until i figure out how to get the objects deleted
properly.

Most of the code below was based on suggestions from:
http://create.tpsitulsa.com/blog/2009/01/31/persistent-handles/
http://create.tpsitulsa.com/blog/2009/01/29/v8-objects/

It's worth noting that the v8 sample code doesn't have any examples of
weak pointers :(.

The class i'm wrapping, PathFinder, can be considered opaque for our
purposes - the binding to the object appears to work, it's just that
my dtor (pf_dtor()) is never called.

The functions listed below are:

- SetupPathFinderClass() sets up the class for JS-side access.
Installs the ctor and class instance prototype
- pf_ctor() - the instance constructor
- pf_dtor() - the weak pointer callback
- pf_wrap() - associates a PathFinder JS object with a C++ object
and does the Persistent/Weak voodoo.

i've tried with and without HandleScopes, but the results are the same
- pf_dtor() is never being called.

The C++ code (JS code follows):


#include <v8/PathFinder.h>
#include <v8.h>
#include <v8/v8-convert.h>
#include <v8/v8-plugin.h>

#include <iostream> /* only for debuggering */
#ifndef CERR
#define CERR std::cerr << __FILE__ << ":" << std::dec << __LINE__ <<
" : "
#endif

namespace v8 { namespace p3 {
using namespace ::v8::bind;
using namespace ::v8::convert;

//static const void * pf_bind_cx() { static const int x=7;return
&x;}

static void pf_dtor(Persistent< Value > object, void *parameter)
{
CERR << "pf_dtor() PathFinder @"<<parameter<<'\n';
PathFinder * pf = static_cast<PathFinder*>( parameter );
//UnbindNative( pf_bind_cx(), parameter, pf );
if( pf != &::v8::p3::plugin::PluginPath() )
{
delete pf;
}
object.Dispose();
object.Clear();
}

static Persistent<Object> pf_wrap( Handle<Object> _self,
PathFinder * value )
{
//HandleScope scope;
//BindNative( pf_bind_cx(), value, value );
Persistent<Object> self( Persistent<Object>::New(_self) );
self.MakeWeak( value, pf_dtor );
self->SetInternalField(0,External::New(value));
self->Set(String::New("bogo"),String::New("bogo was here"));
CERR << "Wrapped PathFinder @"<<value<<" in a Persistent<Object>.\n";
//return scope.Close(self);
return self;
}
static Handle<Value> pf_ctor(const Arguments& argv)
{
if (!argv.IsConstructCall())
return ThrowException(String::New("Cannot call constructor as
function"));

const int argc = argv.Length();
if( (1 == argc) && argv[0]->IsExternal() )
{
External * ex = External::Cast( *argv[0] );
if( ex )
{
return pf_wrap( argv.This(), static_cast<PathFinder*>(ex->Value
()) );
}
else
{
return ThrowException(String::New("First argument to ctor failed
External::Cast()"));
}
}
//HandleScope hscope;
std::string a0 = (argc>0) ? JSToStdString(argv[0]) : "";
std::string a1 = (argc>1) ? JSToStdString(argv[1]) : "";
std::string a2 = (argc>2) ? JSToStdString(argv[2]) : "";
CERR << "PathFinder(["<<a0<<"], ["<<a1<<"], ["<<a2<<"])\n";
PathFinder * pf = new PathFinder( a0, a1, a2 );
//return hscope.Close( pf_wrap( argv.This(), pf ) );
return pf_wrap( argv.This(), pf );
//return hscope.Close( pf_wrap( argv.This(), pf ) );
}

Handle<Value> SetupPathFinderClass(const Handle<Object> target )
{
// Set up prototype:
Handle<FunctionTemplate> ctor = FunctionTemplate::New(pf_ctor);
Local<ObjectTemplate> objt = ctor->InstanceTemplate();
objt->SetInternalFieldCount(1);

// Install ctor:
Handle<Function> pffunc( ctor->GetFunction() );
target->Set(String::New("PathFinder"), pffunc );

// Install instance wrapping PluginPath() shared object:
Handle<Value> pfex( External::New( &::v8::p3::plugin::PluginPath
() ) );
Handle<Object> pfobj = pffunc->NewInstance( 1, &pfex );
pffunc->Set(String::New("plugins"), pfobj );
return pfobj;
}

}} // namespaces


i've created hundreds of PathFinder objects in small scopes to try to
force GC, but my debugging message in pf_dtor() never gets output:

var my = {p:null};
{
for( var i = 0; i < 5; ++i )
{
var p = ['/usr/bin','/bin','~/bin'];
my.p = new PathFinder(p.join(''+i),"",""+i);
//delete my.p;
}
print('my.p=',my.p,typeof my.p,my.p.bogo);
delete my.p;
print('my.p=',my.p);
}
print('my.p=',my.p);
print
('PathFinder.plugins=',PathFinder.plugins,PathFinder.plugins.bogo);
print("Bye!");


The output is:

stephan@jareth:~/cvs/fossil/v8-addons/addons$ m && ./shell js/pf.js
make: Nothing to be done for `default'.
PathFinder-js.cc:43 : Wrapped PathFinder @0xb7c940a0 in a
Persistent<Object>.
PathFinder-js.cc:69 : PathFinder([/usr/bin0/bin0~/bin], [], [0])
PathFinder-js.cc:43 : Wrapped PathFinder @0x88cda78 in a
Persistent<Object>.
PathFinder-js.cc:69 : PathFinder([/usr/bin1/bin1~/bin], [], [1])
PathFinder-js.cc:43 : Wrapped PathFinder @0x88cda40 in a
Persistent<Object>.
PathFinder-js.cc:69 : PathFinder([/usr/bin2/bin2~/bin], [], [2])
PathFinder-js.cc:43 : Wrapped PathFinder @0x88cdc10 in a
Persistent<Object>.
PathFinder-js.cc:69 : PathFinder([/usr/bin3/bin3~/bin], [], [3])
PathFinder-js.cc:43 : Wrapped PathFinder @0x88cdd00 in a
Persistent<Object>.
PathFinder-js.cc:69 : PathFinder([/usr/bin4/bin4~/bin], [], [4])
PathFinder-js.cc:43 : Wrapped PathFinder @0x88cddf0 in a
Persistent<Object>.
my.p= [object Object] object bogo was here
my.p= undefined
my.p= undefined
PathFinder.plugins= [object Object] bogo was here
Bye!
---------------------------------------
(end demo)


:-?

[Alex Iskander]

Hi,

i've created hundreds of PathFinder objects in small scopes to try to
force GC, but my debugging message in pf_dtor() never gets output:

How many of these objects have you created, exactly? I've had to create thousands before it would clear.

You might want to try, to test it:

   for( var i  = 0; i < 50000; ++i )

   {
var p = ['/usr/bin','/bin','~/bin'];
my.p = new PathFinder(p.join(''+i),"",""+i);
//delete my.p;
   }

I'm still looking over the code to see if I can find anything wrong with it, but there isn't anything glaring,

Alex

On Mar 5, 2009, at 1:36 PM, Stephan Beal wrote:

Hi, all!

After having bound hundreds of C-like functions i'm ready to bind my
first class in v8. i've got it working so far except that my Weak
Pointer callback isn't being triggered, and i have been unable to
figure out why. i'm pasting the code here - it's a bit long, but i
would really appreciate any insights on this, as i can't do ANY class-
level wrapping until i figure out how to get the objects deleted
properly.

Most of the code below was based on suggestions from:
 http://create.tpsitulsa.com/blog/2009/01/31/persistent-handles/
 http://create.tpsitulsa.com/blog/2009/01/29/v8-objects/

It's worth noting that the v8 sample code doesn't have any examples of
weak pointers :(.

The class i'm wrapping, PathFinder, can be considered opaque for our
purposes - the binding to the object appears to work, it's just that
my dtor (pf_dtor()) is never called.

Alex Iskander, TPSi

[Ondrej Zara]

Hi Stephan,

I was struggling with a very similar issue; please see http://groups.google.com/group/v8-users/browse_thread/thread/3d4e7e801bd58a0f# for reference.

To make the long story short - V8 by default collects garbage only when necessary; in my case with a simple (empty) testing class, this first happened after creating approx. one to ten million (!) instances.

I would really appreciate some kind of moving forward in this issue. My primary goal is to automatically close DB connections and this is somewhat pressing in my scenario. However, it looks like there is no generic and straightforward way to force GC.


O.

2009/3/5 Stephan Beal <sgb...@googlemail.com>

[Stephan Beal]

On Mar 5, 8:47 pm, Alex Iskander <a...@tpsitulsa.com> wrote:
> How many of these objects have you created, exactly? I've had to  
> create thousands before it would clear.

Sorry, the demo was only 5 objects, but i was trying 1000 at some
point. Even when my JS session exits, the dtor is never called (and i
would expect it to finally be cleaned up when v8 exits the context, at
the latest).

> I'm still looking over the code to see if I can find anything wrong  
> with it, but there isn't anything glaring,

That's comforting :).

Thanks :)

[Alex Iskander]

I too wish there was a way to force garbage collection.

To force destruction of certain objects on program termination, I've had to keep a list of those objects that are resources that need to be explicitly closed.

In addition to explicitly destroying these resources, I also supply v8 with (sometimes fudged) numbers showing the amount of space actually used by each object (the more memory v8 thinks is being used, the more often it garbage collects):

V8::AdjustAmountOfExternalAllocatedMemory(size * 1000);

V8::AdjustAmountOfExternalAllocatedMemory(size * 1000);

Alex

On Mar 5, 2009, at 1:48 PM, Ondrej Zara wrote:

I would really appreciate some kind of moving forward in this issue. My primary goal is to automatically close DB connections and this is somewhat pressing in my scenario. However, it looks like there is no generic and straightforward way to force GC.

Alex Iskander, TPSi

[Alex Iskander]

Hi Stephan,

v8 DOES NOT  clean up when the session exits. This is, of course, very annoying, but apparently is so that, if a web page with dozens of embedded sub-pages (in iframes) is destroyed, there won't be dozens of expensive garbage collections.

In short, the only way to debug it is to create tens of thousands, even, as Ondrej pointed out,  millions, of objects.

Or, you could try adding lines  like this:

// in pf_wrap():

// trick v8 into thinking that 4 kilobytes are being used for this object

V8::AdjustAmountOfExternalAllocatedMemory(1024 * 4);

// in pf_dtor():

// tell v8 we are done with that memory.

V8::AdjustAmountOfExternalAllocatedMemory(-1024 * 4); 

I cover it here, in the v8 cookbook I've been working on in what little spare time I have:

http://create.tpsitulsa.com/wiki/V8/Garbage_Collection

Alex

Alex Iskander, TPSi

[Stephan Beal]

On Mar 5, 8:48 pm, Ondrej Zara <ondrej.z...@gmail.com> wrote:
> I was struggling with a very similar issue; please seehttp://groups.google.com/group/v8-users/browse_thread/thread/3d4e7e80...

Quoting you from that article:

-------
I will probably file this as a bug, because it seems logical to me
that when a JS context is destroyed, all data objects in it should get
GC'ed. What is your opinion?
------

That's a show-stopper bug, IMO. If the dtor is NEVER called, not even
when the context closes down, that's simply a fatal error, IMO. i
can't wrap any classes in good conscience if i know that their dtors
(which are critical components in many C++ idioms) will never, ever,
ever be called. i'm willing to live with indeterminate timing garbage
collection, but the time must be finite - it must happen before v8
simply abandons the object. Without such a guaranty, a Persistent
handle is simply a glorified, guaranteed memory leak.

:-!

[Alex Iskander]

Not exactly a memory leak.

The persistent handle does not belong to the context. It belongs to v8. v8 will collect garbage eventually, and when it does, even if there are no active contexts, that persistent handle's callback will be called.

The only catch is on program termination. Memory will not get leaked, as the operating system will clear all of the memory. Even resources, such as files left open, will be closed. However, with items such as std::ostreams, problems arise because the destructor must be called to flush the file to output; for this, it is required to keep a separate list of all open resources that need to be closed.

I had this problem when trying to write to a file from JavaScript, and created a little helper class that did most of the heavy lifting for me:

http://create.tpsitulsa.com/blog/2009/02/20/v8-application-exit-and-destructors/

Alex

Alex Iskander, TPSi

[Stephan Beal]

On Mar 5, 9:00 pm, Alex Iskander <a...@tpsitulsa.com> wrote:
> Hi Stephan,
> v8 DOES NOT  clean up when the session exits. This is, of course, very  
> annoying, but apparently is so that, if a web page with dozens of  
> embedded sub-pages (in iframes) is destroyed, there won't be dozens of  
> expensive garbage collections.

And instead of 10ms of gc there are dozens of frame objects left
danging in memory. :/ v8 should not, as a generic library, be making
the decision to not GC based solely on the characteristics of one
application (or even one application domain - the web).

> V8::AdjustAmountOfExternalAllocatedMemory(1024 * 4);

Eeek.

> I cover it here, in the v8 cookbook I've been working on in what  
> little spare time I have:http://create.tpsitulsa.com/wiki/V8/Garbage_Collection

Thank you very much - i'll have a look immediately.

[Stephan Beal]

On Mar 5, 8:47 pm, Alex Iskander <a...@tpsitulsa.com> wrote:
> How many of these objects have you created, exactly? I've had to  
> create thousands before it would clear.

> You might want to try, to test it:
>     for( var i  = 0; i < 50000; ++i )

That did it, actually. Now i'm seeing:

PathFinder-js.cc:24 : pf_dtor() PathFinder @0x9213378
PathFinder-js.cc:24 : pf_dtor() PathFinder @0x9213288
PathFinder-js.cc:24 : pf_dtor() PathFinder @0x9213198
PathFinder-js.cc:24 : pf_dtor() PathFinder @0x92130a8
PathFinder-js.cc:24 : pf_dtor() PathFinder @0x9212fb8
PathFinder-js.cc:24 : pf_dtor() PathFinder @0x9212ec8
PathFinder-js.cc:24 : pf_dtor() PathFinder @0x9212dd8

But the fact that it doesn't clean up at shutdown really pisses me
off, to be frank (delayed indefinitely is fine, as long as
"indefinitely" ends during v8's shutdown process). That rules out
whole categories of objects which can't safely be wrapped. It also
plays havok objects allocated via plugins, because the main
application needs a way to know which objects were allocated (but not
yet destroyed by v8).

Your suggestion from the other thread:

//on object creation
autoDestroy(theObject);
//on object deletion
cancelAutoDestroy(theObject);

is looking to be my only sensible way around the problem at the moment
- feed all objects to an app-maintained GC pool.

Bummer. And i had such high hopes for this evening's hacking session.
Instead i've got to add "supplemental" GC to v8.

Thanks again to you and Ondrej for your quick and informative help :).

[Alex Iskander]

It annoyed me too. I wish they at least had a V8::Shutdown() function, for end-of-program.

Good luck on your hacking session. Hopefully the supplemental GC doesn't take you too long.

On Mar 5, 2009, at 2:18 PM, Stephan Beal wrote:

Bummer. And i had such high hopes for this evening's hacking session.
Instead i've got to add "supplemental" GC to v8.

Alex Iskander, TPSi

[Stephan Beal]

On Mar 5, 9:22 pm, Alex Iskander <a...@tpsitulsa.com> wrote:
> It annoyed me too. I wish they at least had a V8::Shutdown() function,  
> for end-of-program.

i just suggested such a feature here:

http://code.google.com/p/v8/issues/detail?id=256

though my suggestion was a callback registration for context shutdown,
like:

typedef void (*ContextShutdownCallback)( Context *, void * arg )

(or similar)

and we register:

Context::GetCurrent()->AddShutdownHandler( myCallback, myData )


A more general solution might be:

void Object::AddShutdownHandler( void (*)( Object *, void *
userData ) );

That could be used for any objects, not just Weak ones. However,
having such a feature also makes MakeWeak()'s argument irrelevant, as
client side we could achieve the cleanup solution more generically via
this (or a similar) new interface.

> Good luck on your hacking session. Hopefully the supplemental GC  
> doesn't take you too long.

Your suggested approach gave me some ideas which can be done fairly
generically with templates, i think.

:)

[Stephan Beal]

i've just hobbled together a generic solution based on your idea of
adding/removing from a client-side GC pool. The implementation isn't
all too long, so i'll go ahead and paste it here in case anyone else
might get some use out of it (it should compile as-is). With this in
place, my PathFinder objects are getting reliably deleted (a bit late,
but better than never).

Thanks again for your help!

//v8-cleanup.h
#ifndef WANDERINGHORSE_NET_GOOGLE_V8_CLEANUP_H_INCLUDED_
#define WANDERINGHORSE_NET_GOOGLE_V8_CLEANUP_H_INCLUDED_ 1

namespace v8 { namespace p3 {
/**
"Supplemental", client-side garbage collection routines, to work
around certain deficiencies in v8's GC (namely that it's never
guaranteed
to be called by the time the engine shuts down. Ever.).
*/
namespace cleanup {

/**
Typedef for generic destructor functions. They are semantically
compatible with free(), though whether or not they actually
deallocate any memory is an internal implementation detail
(e.g. they may use an object pool).
*/
typedef void (*Destructor)( void * obj );

namespace Detail
{
/**
Helper to generate destructor functions halfway type-safely.
*/
template <typename T>
struct DestructorGen
{
static void dtor( void * obj )
{
T * t = obj ? static_cast<T*>( obj ) : 0;
if( t )
{
delete t;
}
}
};
}

/**
Client apps can create one of these in a scope right above the
scope in which their JS Context lives. The idea is that the context
is allowed to go out of scope, and only then do we try to clean up.
This allows us to go through the normal v8 channels up until the
last dying moments of the context, and if the context isn't cleaned
up, we can take over where it left off.

It is never a good idea to have more than one of these active
at any time, as that will cause unwanted, premature deletions.
Only use one per application (not per JS context!).
*/
class CleanupSentry
{
public:
/** Does nothing. */
CleanupSentry();
/**
Calls Cleanup()
*/
~CleanupSentry();
};

/**
Adds the given object to "cleanup list", such that
Cleanup() will destroy obj by calling dtor(obj).
*/
void AddToCleanup( void * obj, Destructor dtor );

/**
Adds obj to the cleanup list (see the 2-arg overload
of this function). The destructor simply calls
(delete obj), so that must be legal for T.
*/
template <typename T>
void AddToCleanup( T * obj )
{
AddToCleanup( obj, Detail::DestructorGen<T>::dtor );
}

/**
Removes the given object from the cleanup list,
so Cleanup() will not destroy it.
*/
void RemoveFromCleanup( void const * obj );

/**
Destroys all objects which have been added by AddToCleanup()
and clears
the internal cleanup list.
*/
void Cleanup();

}}} // namespaces

#endif /* WANDERINGHORSE_NET_GOOGLE_V8_CLEANUP_H_INCLUDED_ */


//v8-cleanup.cc
#include <v8/v8-cleanup.h>

#include <map>

namespace v8 { namespace p3 { namespace cleanup {

typedef std::pair<void *, Destructor> GCEntry;
typedef std::map<void const *,GCEntry> GCMap;

static GCMap & gcList()
{
static GCMap bob;
return bob;
}

/**
A sentry object to call Cleanup() during the static destruction
phase. This may be called after v8 has already been wiped out.
We can only hope that gcList()'s object is still valid at this
point, too.
*/
// nonono. too risky vis-a-vis post-main() destruction order
// of shared containers and such.
// static CleanupSentry cleanupSentry = CleanupSentry();

CleanupSentry::~CleanupSentry()
{
Cleanup();
}
CleanupSentry::CleanupSentry()
{
}

void AddToCleanup( void * obj, Destructor dtor )
{
gcList().insert( std::make_pair( obj, std::make_pair(obj,dtor) ) );
}

void RemoveFromCleanup( void const * obj )
{
gcList().erase(obj);
}

void Cleanup()
{
GCMap cp;
cp.swap( gcList() );
GCMap::iterator it = cp.begin();
GCMap::iterator ed = cp.end();
for( ; it != ed; ++it )
{
Destructor d = it->second.second;
void * v = it->second.first;
if(0 != d) d(v);
}
}

}}}