481444 - TM: objectHook is called on trace, via js_NewObject [@ LeaveTree] or [@ js_SynthesizeFrame]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

Prettiest crash report I've ever seen:

http://crash-stats.mozilla.com/report/index/bb195590-343b-4abc-8ea3-acde22090304

Comment 1

[John J. Barton]

Someone cced me, but I don't see Firebug involved. We don't use jsd_Objecthook.

Comment 2

[Daniel Einspanjer [:dre] [:deinspanjer]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090304 Minefield/3.2a1pre

Jason asked me if I had Firebug installed.  I do, but it is normally not active when the crash occurs.

I hit what feels like this crash about twice a day.  Normally, I Cmd-Tab into Minefield from some other app, interact with something in the browser (either a chrome widget, or a link or button on a webpage, or I use a hotkey like Cmd-W) and it crashes.  I restart, restore the session, and then immediately perform the same action I was attempting before and it works fine.

If there is any other information I can provide, just let me know.

Comment 3

[Jason Orendorff [:jorendorff]]

johnjbarton: I suspect this is a firebug-only crash, since it happens in jsd. Still looking at it though. If firebug always sets the DISABLE_OBJECT_TRACE flag, I might have to look elsewhere.

Either way the bug is definitely in the JS engine, but I thought you might be interested.

Trying to figure out what we want to do here.  Note that in all these cases it would be fine to call these hooks from a _FAIL builtin (modulo bug 463153).  They'll just bail off trace at some point.

debuggerHandler: Can't be called on trace - we don't trace JSOP_DEBUGGER.

sourceHandler: Same - we don't trace JSOP_EVAL or Function().

executeHook: Same - dominated by a call to js_GetTopStackFrame.

throwHook: Same - it's not called when the exception is thrown but from the error: label in the interpreter.

callHook: Different. We just don't bother calling it if we're on trace. There's a special case in the tracer for this too; nothing too complicated.

debugErrorHook: Hmmm. Unlike throwHook this fires when an error is reported. I think there are places where this can happen to us on trace, but I'm not sure.

If it's ok to just skip calling objectHook on trace, that's easy enough.  I'm a bit more worried about debugErrorHook.

Comment 4

[John J. Barton]

(In reply to comment #3)
> johnjbarton: I suspect this is a firebug-only crash, since it happens in jsd.
> Still looking at it though. If firebug always sets the DISABLE_OBJECT_TRACE
> flag, I might have to look elsewhere.

http://code.google.com/p/fbug/source/browse/branches/firebug1.4/components/firebug-service.js#874

> 
> Either way the bug is definitely in the JS engine, but I thought you might be
> interested.
> 
> Trying to figure out what we want to do here.  Note that in all these cases it
> would be fine to call these hooks from a _FAIL builtin (modulo bug 463153). 
> They'll just bail off trace at some point.
> 
> debuggerHandler: Can't be called on trace - we don't trace JSOP_DEBUGGER.

I'm unsure what you mean:
  1) If the jit sees 'debugger', it marks the trace as do-not-jit-compile, or
  2) 'debugger' is a no-op on jit-ed code.
I guess #1 since its the only one that make sense.

> 
> sourceHandler: Same - we don't trace JSOP_EVAL or Function().

Ouch. dojo will be sad.

> 
> executeHook: Same - dominated by a call to js_GetTopStackFrame.
> 
> throwHook: Same - it's not called when the exception is thrown but from the
> error: label in the interpreter.
> 
> callHook: Different. We just don't bother calling it if we're on trace. There's
> a special case in the tracer for this too; nothing too complicated.

So there must be some new API to say "yes I really want to trap function calls".

> 
> debugErrorHook: Hmmm. Unlike throwHook this fires when an error is reported. I
> think there are places where this can happen to us on trace, but I'm not sure.
> 
> If it's ok to just skip calling objectHook on trace, that's easy enough.  I'm a
> bit more worried about debugErrorHook.

While firebug does not user objectHook, if we did then JIT would be great because we will probably do lots of cycles. On the other hand if we take debugErrorHook, we already interrupted something and we are likely to do I/O or UI work so JIT is not likely to help.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

My duped bug 481519 is the topcrasher #3 on 1.9.1. There are a couple of crash reports for 3.1b3 already listed. But only two comments were given:

* Went to nytimes.com
* These random crashes have been happening in today's build, possibly yesterday's. (3/2 and 3/3)

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comments given for crashes with the LeaveTree signature:

* Went to nytimes.com, hit cmd-l to go somewhere else while it was still loading
* try to inspect page DOM with firebug
* the lastest builds have been crashing a lot over the last two week relative to the prior builds. I'm not doing much different in my browsing behavior.
* news.bbc.co.uk
* Navigating back and forth
* Bank of America, clicked link to Log In.

http://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A3.1b3&version=Firefox%3A3.1b3pre&query_search=signature&query_type=exact&query=LeaveTree&date=&range_value=1&range_unit=weeks&do_query=1&signature=LeaveTree

Comment 8

[Andreas Gal :gal]

I am not getting source links for any of the bug reports? Is there anything we can do about that?

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

So the numbers behind the source and header files don't help you?

0  	libmozjs.dylib  	LeaveTree  	js/src/jstracer.h:334
1 	libmozjs.dylib 	js_DeepBail 	js/src/jstracer.cpp:4679
2 	libmozjs.dylib 	JS_FrameIterator 	js/src/jscntxt.h:1418

Comment 10

[Andreas Gal :gal]

The files change frequently. There used to be a clickable link directly to the corresponding version. What happened to that?

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

This is bug 470841 and not implemented yet. We have to check the hg log and compare it with the build id so we will know which version it matches. Hard handwork..

Comment 12

[amau96]

I got the same bug, very often with last nightly of shireteko
http://crash-stats.mozilla.com/report/index/5a4ab764-a29d-4d67-b166-03ccb2090304
http://crash-stats.mozilla.com/report/index/a780fb98-e5a4-414a-a58b-83c412090304
http://crash-stats.mozilla.com/report/index/642b827c-ff68-422a-bbe4-0fae52090306
and this one, but not sure it's the same
http://crash-stats.mozilla.com/report/index/8d643d4f-2e8e-4014-8b28-6a7c42090306

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

(In reply to comment #12)
> I got the same bug, very often with last nightly of shireteko

Any steps to reproduce this crash?

Comment 14

[amau96]

I don't understand when it crashes. I reopen shiretoko after a crash, same pages in same tabs and no crash..

Comment 15

[Daniel Einspanjer [:dre] [:deinspanjer]]

I believe this crash is tied in some way to Firebug.  I currently have Firebug 1.4.0a12 installed.  I was getting on average about two crashes a day when I had it enabled.  I disabled it the day this bug was created and I had zero crashes until yesterday when I enabled it again briefly to use the Net tab on a site.  I forgot to disable it after I was done, and a few hours later I got a crash.  I disabled it again and I have not had any further crashes.

Comment 16

[Andreas Gal :gal]

This is danderson's missing global slots patch (judging from the backtrace).

Comment 17

[Jason Orendorff [:jorendorff]]

I think the summary is right and comment 16 is wrong.  Without a reproducible test case all we can do is check in one fix or the other, and see if it goes away.

Comment 18

[Jason Orendorff [:jorendorff]]

Attachment: v1

This hook is called in one other place, js_FinalizeObject, which can only be called from GC and thus is never on trace.

debugErrorHook should never be called on trace.  Hmm-- I guess I could add JS_ASSERT_NOT_ON_TRACE(cx) to all the ReportError functions; but maybe it's redundant, since we're likely to hit the cx->bailExit assertion anyway.

Bug 482958 aims to kill this whole class of bugs.

Comment 19

[Jason Orendorff [:jorendorff]]

 JS_ASSERT_NOT_ON_TRACE(cx);

would be the wrong thing-- I meant:

  JS_ASSERT(!JS_ON_TRACE(cx) || cx->bailExit);

Comment 20

[Jason Orendorff [:jorendorff]]

http://hg.mozilla.org/tracemonkey/rev/cbf2e23a6e58

I claim this is now fixed-in-tracemonkey.  Comment here if not!

Comment 21

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/cbf2e23a6e58

Comment 22

[Aakash Desai [:aakashd]]

This was really odd, but this was occurring on build: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.9.1b4pre) Gecko/20090331 Shiretoko/3.5b4pre ID:20090331041754

with the following crash reports (i've got more if needed): 

http://crash-stats.mozilla.com/report/index/6fdb1d65-7bec-4456-a89b-4e1462090406?p=1

http://crash-stats.mozilla.com/report/index/0bc4e928-1046-45af-80e1-5b1402090331


...but, its working fine now with Build: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090402 Shiretoko/3.5b4pre ID:20090402045633

Comment 23

[Ryan VanderMeulen [:RyanVM]]

What was the last changeset on the 3/31 build? The fix for this bug landed on the 1.9.1 branch on Tue Mar 31 15:02:07 2009 -0700 according to the 1.9.1 pushlog.

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

(In reply to comment #23)
> What was the last changeset on the 3/31 build? The fix for this bug landed on
> the 1.9.1 branch on Tue Mar 31 15:02:07 2009 -0700 according to the 1.9.1
> pushlog.

Why is it not marked as fixed1.9.1 then?

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

No more reported crashes on trunk and 1.9.1 the last weeks for all platforms. Marking as verified.