weak_callback() not calling Dispose() or ClearWeak()?

[GoogleCodeExporter]

What steps will reproduce the problem?

It appears that when I wrap my class, load a *huge* script (I fill it mostly 
with comments), and execute it. It happens particularly when I dynamically 
create a NewInstance() of my class, and return it to JS in a function called 
from JS, during the second execution of the same script source (which I compile 
again each time).

It tends not to happen with a lesser sized script, which leads me to assume 
that this bug will only be apparent if/when the GC is forced to collect. 

What is the expected output? What do you see instead?

v8 crashes with:

#
# Fatal error in <edited out>/src/global-handles.cc, line 213
# CHECK(state_ != NEAR_DEATH) failed
#

What version of the product are you using? On what operating system?

v8: ~< r8053
juice: r1332

Please provide any additional information below.

I apologize for not showing reproducible code; my use case is very complex, and 
large, reducing it would take lots of time. Thus I will first try pointing out 
where I suspect the problem is.

I believe that, in ClassWrap.h, the weak_callback function fails to call 
ClearWeak(), or Dispose() in certain paths, which is a requirement (and a 
common cause for that v8 assertion, apparently). By placing a pv.Dispose(); 
pv.Clear(); line at the end of the function, it seemed to fix the problem, 
though I don't know if this is a total kludge. I am pointing this out so that 
someone might take a look and see the obvious. It is entirely possible that I 
am missing something, and that my own code is misusing/abusing v8/juice etc. 
and I am at fault. If it is not obvious that weak_callback is at fault, I will 
make an attempt at reducing my code so that I can post something reproducible.

Original issue reported on code.google.com by fst...@gmail.com on 26 May 2011 at 12:06

[GoogleCodeExporter]

Hi! Is this on a 64-bit platform, by chance? i have seen this problem but only 
on 64-bit, and i cannot explain why it happens. If i call Dispose() then the 
weak callback is being called twice, which is just as bad as the current 
behaviour.

See this thread for more details:

http://groups.google.com/group/v8-users/browse_thread/thread/582f506169f5648f/bc
23c61aaa45b2bc?lnk=gst&q=NEAR_DEATH#bc23c61aaa45b2bc

Coincidentally, i tried to re-test this again last night, but neither the v8 
trunk nor the bleeding-edge branch builds fully on x86/64 at the moment.

Original comment by sgbeal@googlemail.com on 26 May 2011 at 7:50

[GoogleCodeExporter]

Original comment by sgbeal@googlemail.com on 26 May 2011 at 7:53

• Added labels: Severity-ShowStopper

[GoogleCodeExporter]

Original comment by sgbeal@googlemail.com on 26 May 2011 at 10:10

• Added labels: Priority-Critical
• Removed labels: Priority-Medium

[GoogleCodeExporter]

Sorry for all the one-line status changes. i keep forgetting to set certain 
fields. This bug is currently #1 on my bug-fix list, but i can't promise that i 
will get a chance to look at it more closely before the weekend. And i need to 
find a rev of v8 which builds on my 64-bit box.

Original comment by sgbeal@googlemail.com on 26 May 2011 at 10:11

• Changed state: Accepted

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Interesting, its a 32 bit build on my end, but I am on a 64 bit machine. Also, 
I am basically building it myself from scratch, and skipping the build system 
that comes with v8, so that may potentially be causing additional problems.

Funny, that ML post was where I got the notion that it was either ClearWeak(), 
or Dispose() in the callback, yet I didn't notice you were the complainant :).

Original comment by fst...@gmail.com on 26 May 2011 at 12:31

[GoogleCodeExporter]

Hi! i don't suspect that your custom build is the cause of this. i've been 
seeing this behaviour intermittently for a couple months but haven't found the 
energy to track it down. Writing a small test-case for it has proven 
challenging because it's built off of so much code. On my 32-bit laptop it 
seems to pose no problems, but on my main machine i'm seeing the NEAR_DEATH 
assertion.

Original comment by sgbeal@googlemail.com on 26 May 2011 at 4:59

[GoogleCodeExporter]

Just FYI: i've been trying several combinations of Clear()/Dispose() and i'm 
still not getting the behaviours i would expect. In my current setup, i'm 
seeing identical behaviours regardless of whether i call (Dispose+Clear), (just 
Dispose), or (just Clear): the NEAR_DEATH assertion. It doesn't seem to matter 
if i call them before or after the actual cleanup bits.

:(

PS: the easiest way to test this at the moment is to use the demo app in the 
'convert' source tree. It uses a derivative of ClassWrap and i can reproduce 
this problem 100% of the time there (but only on 64-bit machines - on my 32-bit 
machine it behaves exactly how i would expect it to).

Original comment by sgbeal@googlemail.com on 28 May 2011 at 12:33

[GoogleCodeExporter]

i believe this is fixed in r1337 (in the 'edge' branch), but i was never able 
to reproduce this behaviour directly from ClassWrap (only from its very close 
derivative in the v8::convert source tree). While testing i also finally found 
a workaround for the crash condition in the derivative code.

Thank you very much for the report. :) i'll leave this ticket open for the time 
being, but will close it in a few days if i don't hear anything regarding your 
results. If you report that it works for you i'll get this moved into the trunk 
branch.

Happy Hacking!

Original comment by sgbeal@googlemail.com on 28 May 2011 at 1:04

[GoogleCodeExporter]

Ok, I just got a chance to try out the 'edge' branch; it seems not to crash in 
as it did. I can't say I have done a thorough test for anything else. All I 
*can* say is that it doesn't crash in the single instance that I was using to 
crash trunk. Thanks for this. Incidentally, where should I put a link to my 
project?

Original comment by fst...@gmail.com on 1 Jun 2011 at 7:54

[GoogleCodeExporter]

That's great news :). Post your project's link here or send it to me per mail 
and i'll add it to the wiki if you like.

i will get this merged into the trunk branch sometime in the next couple of 
days.

Thanks again for taking the time to report the problem (and a bigger thanks for 
localizing the problem).

:)

Original comment by sgbeal@googlemail.com on 1 Jun 2011 at 2:21

[GoogleCodeExporter]

Take a look and tell me what you think :D.

https://github.com/realazthat/v8.rocket

Original comment by fst...@gmail.com on 1 Jun 2011 at 2:25

[GoogleCodeExporter]

The idea of a renderer-agnostic HTML/CSS engine sounds really interesting. i'll 
get a list of links set up in the next couple of days (tomorrow is a holiday 
here).

If you are only using juice for the class binding then porting from v8::juice 
to v8::convert might be a good idea for your project. In short v8::convert is 
basically a trimmed-down version of juice which contains only the type 
conversions API and one class-binding helper (based off of ClassWrap). Most 
importantly, it's a header-only impl, which would make it trivial to include 
directly in to your project.

http://code.google.com/p/v8-juice/wiki/V8Convert

Original comment by sgbeal@googlemail.com on 1 Jun 2011 at 2:34

• Changed state: Verified

[GoogleCodeExporter]

Good idea. For now I'll leave it as is, because I have it down to a science. 
But if/when I do a refactor, I'll definately try v8::convert ... I am a big fan 
of header libs for C++, and with template heavy code, I am even a bigger fan.

Original comment by fst...@gmail.com on 1 Jun 2011 at 2:44

[GoogleCodeExporter]

i haven't seen this happen in the past couple weeks, so i'm going to call it 
fixed. Reminder to self: this isn't in the trunk yet. i need to sync with Anton 
and pull in some of his changes before i push it to the trunk.

Original comment by sgbeal@googlemail.com on 15 Jun 2011 at 1:57

• Changed state: Fixed