Lifespan of php_v8js_ctx

[cscott]

I'd like to think about the cleanup of php_v8js_ctx some more. I think it might be possible for the ctx to be deallocated before all of the objects created in that context have been dereferenced -- like if you return a javascript object to PHP, store it in a property somewhere, and then the V8JS object falls out of scope. Any ideas on how best to handle that situation? Maybe we should ref the V8JS object whenever we create a new PHP wrapper of a JS object? V8 also has some "object group" features which might be relevant.

[satoshi75nakamoto]

Definitely worth exploring for sure. @stesie — what do you think?

Also for what it's worth I've added both @cscott and @stesie as contributors to this repo.

[cscott]

Yeah, I confirmed it's a problem (see above test, which segfaults). Working on a solution...

[satoshi75nakamoto]

@cscott — is this currently fixed?

[cscott]

Nope, it's still on my todo list.
On Oct 27, 2013 1:37 PM, "Patrick Reilly" notifications@github.com wrote:

@cscott https://github.com/cscott — is this currently fixed?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/64#issuecomment-27174252
.

[satoshi75nakamoto]

Okay thanks for the status update.

[stesie]

I'm not sure which approach is better, keeping references on the V8Js object so the V8 context isn't disposed unless the last reference to a V8 handle is destroyed ... or the other way round, make all uses of V8 handles kept in PHP context throw if the V8Js object is destroyedi

I like to think of the V8Js object of a handle to the V8 isolate/context and thus, if I don't keep a ref on it, it feels natural that i cannot use kept references on handles anymore. This way it's especially easy to be sure the V8 instance is dead (and I don't accidentally have a handle around anymore)

[cscott]

I think I'd like to add a getContext() method to the V8Object/V8Function class that returns the V8Js object which created it. That will make it easier to store V8 objects in fields and then later use them in an executeString() call. (It would be nice if there were also an associative array of 'parameters' passed to executeString that were stored in a local scope, so you didn't need to add temporary properties to the V8Js object itself in order to pass data to executeString.) While I'm adding methods, there might be a __toString method as well which invokes the native V8 toString().

Anyway, V8Object aka php_v8js_object implicitly carries around a reference to the V8Js anyway (via its isolate, which contains a pointer to the php_v8js_ctx) so IMO we might as well make that explicit and refcounted.

[stesie]

I was just revisiting the V8Js issue list and stumbled across this one. This has been fixed meanwhile, however by invalidating all v8 objects upon destruction of the last handle to the V8Js object. Every further access then yields an exception.

I'll merge your test now, since it's generall a good thing, but I'll adapt it to the current behaviour (i.e. check for exceptions and no crashing of course)

cheers
~stesie