629285 - Firefox 4.0b10 Crash [@ AsyncGetBookmarksForURI<void (nsNavBookmarks::*)(mozilla::places::ItemChangeData), mozilla::places::ItemChangeData>::HandleResult ][@ mozilla::places::ItemChangeData::ItemChangeData(mozilla::places::ItemChangeData const&)]

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Marcia Knous [:marcia]]

Seen while reviewing crash stats. New Mac only crash which is so far low volume. http://tinyurl.com/4dn7uy2 to the reports.

Frame 	Module 	Signature [Expand] 	Source
0 	XUL 	AsyncGetBookmarksForURI<void ,mozilla::places::ItemChangeData>::HandleResult 	toolkit/components/places/src/nsNavBookmarks.cpp:148
1 	XUL 	mozilla::storage::::CallbackResultNotifier::Run 	storage/src/mozStorageAsyncStatementExecution.cpp:100
2 	XUL 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:633
3 	XUL 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:208
4 	XUL 	XPCWrappedNative::CallMethod 	js/src/xpconnect/src/xpcwrappednative.cpp:3072
5 	XUL 	XPC_WN_CallMethod 	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1593
6 	XUL 	js::Interpret 	js/src/jscntxtinlines.h:692
7 	XUL 	js::Invoke 	js/src/jsinterp.cpp:657
8 	XUL 	js_fun_apply 	js/src/jsfun.cpp:2182
9 	XUL 	js::Interpret 	js/src/jscntxtinlines.h:692
10 	XUL 	js::Invoke 	js/src/jsinterp.cpp:657
11 	XUL 	js::ExternalInvoke 	js/src/jsinterp.cpp:858
12 	XUL 	JS_CallFunctionValue 	js/src/jsinterp.h:961
13 	XUL 	nsXPCWrappedJSClass::CallMethod 	js/src/xpconnect/src/xpcwrappedjsclass.cpp:1700
14 	XUL 	nsXPCWrappedJS::CallMethod 	js/src/xpconnect/src/xpcwrappedjs.cpp:588
15 	XUL 	PrepareAndDispatch 	xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_darwin.cpp:153
16 	XUL 	XUL@0xe1b69a 	
17 	XUL 	nsHttpChannel::OnDataAvailable 	netwerk/protocol/http/nsHttpChannel.cpp:4109
18 	XUL 	nsInputStreamPump::OnStateTransfer 	netwerk/base/src/nsInputStreamPump.cpp:510
19 	XUL 	nsInputStreamPump::OnInputStreamReady 	netwerk/base/src/nsInputStreamPump.cpp:400
20 	XUL 	nsInputStreamReadyEvent::Run 	xpcom/io/nsStreamUtils.cpp:112
21 	XUL 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:633
22 	XUL 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:208
23 	XUL 	XPCWrappedNative::CallMethod 	js/src/xpconnect/src/xpcwrappednative.cpp:3072
24 	XUL 	XPC_WN_CallMethod 	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1593
25 	XUL 	js::Interpret 	js/src/jscntxtinlines.h:692
26 	XUL 	js::Invoke 	js/src/jsinterp.cpp:657
27 	XUL 	js_fun_apply 	js/src/jsfun.cpp:2182
28 	XUL 	js::Interpret 	js/src/jscntxtinlines.h:692
29 	XUL 	js::Invoke 	js/src/jsinterp.cpp:657
30 	XUL 	js_fun_call 	js/src/jsfun.cpp:2115
31 	XUL 	js::Interpret 	js/src/jscntxtinlines.h:692
32 	XUL 	js::Invoke 	js/src/jsinterp.cpp:657
33 	XUL 	js::ExternalInvoke 	js/src/jsinterp.cpp:858
34 	XUL 	JS_CallFunctionValue 	js/src/jsinterp.h:961
35 	XUL 	nsXPCWrappedJSClass::CallMethod 	js/src/xpconnect/src/xpcwrappedjsclass.cpp:1700
36 	XUL 	nsXPCWrappedJS::CallMethod 	js/src/xpconnect/src/xpcwrappedjs.cpp:588
37 	XUL 	PrepareAndDispatch 	xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_darwin.cpp:153
38 	XUL 	XUL@0xe1b69a 	
39 	XUL 	nsNavHistory::RunInBatchMode 	toolkit/components/places/src/nsNavHistory.cpp:4043
40 	XUL 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:208
41 	XUL 	XPCWrappedNative::CallMethod 	js/src/xpconnect/src/xpcwrappednative.cpp:3072
42 	XUL 	XPC_WN_CallMethod 	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1593
43 	XUL 	js::Interpret 	js/src/jscntxtinlines.h:692
44 	XUL 	js::Invoke 	js/src/jsinterp.cpp:657
45 	XUL 	js_fun_apply 	js/src/jsfun.cpp:2182
46 	XUL 	js::Interpret 	js/src/jscntxtinlines.h:692
47 	XUL 	js::Invoke 	js/src/jsinterp.cpp:657
48 	XUL 	js_fun_call 	js/src/jsfun.cpp:2115
49 	XUL 	js::Interpret 	js/src/jscntxtinlines.h:692
50 	XUL 	js::Invoke 	js/src/jsinterp.cpp:657
51 	XUL 	js_fun_call 	js/src/jsfun.cpp:2115
52 	XUL 	js::Interpret 	js/src/jscntxtinlines.h:692
53 	XUL 	js::Invoke 	js/src/jsinterp.cpp:657
54 	XUL 	js_fun_call 	js/src/jsfun.cpp:2115
55 	XUL 	js::Interpret 	js/src/jscntxtinlines.h:692
56 	XUL 	js::Invoke 	js/src/jsinterp.cpp:657
57 	XUL 	js_fun_apply 	js/src/jsfun.cpp:2182
58 	XUL 	js::Interpret 	js/src/jscntxtinlines.h:692
59 	XUL 	js::Invoke 	js/src/jsinterp.cpp:657
60 	XUL 	js_fun_call 	js/src/jsfun.cpp:2115
61 	XUL 	js::Interpret 	js/src/jscntxtinlines.h:692
62 	XUL 	js::Invoke 	js/src/jsinterp.cpp:657
63 	XUL 	js_fun_call 	js/src/jsfun.cpp:2115
64 	XUL 	js::Interpret 	js/src/jscntxtinlines.h:692
65 	XUL 	js::Invoke 	js/src/jsinterp.cpp:657
66 	XUL 	js::ExternalInvoke 	js/src/jsinterp.cpp:858
67 	XUL 	JS_CallFunctionValue 	js/src/jsinterp.h:961
68 	XUL 	nsXPCWrappedJSClass::CallMethod 	js/src/xpconnect/src/xpcwrappedjsclass.cpp:1700
69 	XUL 	nsXPCWrappedJS::CallMethod 	js/src/xpconnect/src/xpcwrappedjs.cpp:588
70 	XUL 	PrepareAndDispatch 	xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_darwin.cpp:153
71 	XUL 	XUL@0xe1b69a 	
72 	XUL 	nsTimerImpl::Fire 	xpcom/threads/nsTimerImpl.cpp:428
73 	XUL 	nsTimerEvent::Run 	xpcom/threads/nsTimerImpl.cpp:517
74 	XUL 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:633
75 	XUL 	NS_ProcessPendingEvents_P 	nsThreadUtils.cpp:200
76 	XUL 	nsBaseAppShell::NativeEventCallback 	widget/src/xpwidgets/nsBaseAppShell.cpp:135
77 	XUL 	nsAppShell::ProcessGeckoEvents 	widget/src/cocoa/nsAppShell.mm:405
78 	CoreFoundation 	CoreFoundation@0x4e400 	
79 	CoreFoundation 	CoreFoundation@0x4c5f8

Comment 1

[Marco Bonardo [:mak]]

taking for investigation

Comment 3

[Marco Bonardo [:mak]]

See https://bugzilla.mozilla.org/show_bug.cgi?id=629636#c0 for an interesting stack too.

Comment 4

[Shawn Wilsher :sdwilsh]

(In reply to comment #3)
> See https://bugzilla.mozilla.org/show_bug.cgi?id=629636#c0 for an interesting
> stack too.
Huh, that seems to implicate Sync here...

Comment 5

[Marco Bonardo [:mak]]

(In reply to comment #4)
> (In reply to comment #3)
> > See https://bugzilla.mozilla.org/show_bug.cgi?id=629636#c0 for an interesting
> > stack too.
> Huh, that seems to implicate Sync here...

I think it's unrelated, it's just a visit addition that ends up calling this code (due to the onVisit observer).

Comment 6

[Marco Bonardo [:mak]]

and in that bug there is this link
http://crash-stats.mozilla.com/report/list?product=Firefox&range_value=4&range_unit=weeks&signature=mozilla%3A%3Aplaces%3A%3AItemChangeData%3A%3AItemChangeData%28mozilla%3A%3Aplaces%3A%3AItemChangeData%20const%26%29

some Windows crash too, so this is not mac only, it's just they crash at different stack positions.

Comment 7

[Marco Bonardo [:mak]]

due to the above (this crash being cross-platform and a regression) I think I'm going to ask for soft-blocking on it.

Comment 8

[Scoobidiver (away)]

It is #12 top crasher on Mac OS X in 4.0b10 over the last week.

Comment 9

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I can reproduce this by deleting a history item. I open Show All History, double click on Today and then delete the entry for Slashdot that I have in there. (The actual URL is probably not relevant)

Other items delete fine. It just just this one.

Comment 10

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

See crash report:

https://crash-stats.mozilla.com/report/index/814393e5-87ea-414b-a567-d2d852110202

Comment 11

[Marco Bonardo [:mak]]

thanks, will try to reproduce that way.

Comment 12

[Marco Bonardo [:mak]]

(In reply to comment #9)
> I can reproduce this by deleting a history item. I open Show All History,
> double click on Today and then delete the entry for Slashdot that I have in
> there. (The actual URL is probably not relevant)
> 
> Other items delete fine. It just just this one.

Are you able to crash every time you do that operation, or only sometimes? I really can't find a way to crash :(
If you can reproduce reliably, maybe I could give you a trybuild to test.

Comment 13

[Marco Bonardo [:mak]]

another interesting and related stack: https://crash-stats.mozilla.com/report/index/89fea315-eeea-41c7-8c81-09d2d2110127
the URIBinder crashes trying to dereference the uri.

Comment 14

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

> Are you able to crash every time you do that operation, or only sometimes? I
> really can't find a way to crash :(
> If you can reproduce reliably, maybe I could give you a trybuild to test.

Yeah this is still totally reproducable for me. I've made backup of my profile directory to be sure. I'm more than happy to play with trybuilds if that gives us more info.

Comment 15

[Marco Bonardo [:mak]]

yes please, preserve your environment, I will link here a trybuild and you can test it.
Could you also try to create a new profile, copy your places.sqlite file from the saved profile to that one, and try to crash with the new profile? I'd be interested in knowing if effectively contents of that entry do matter (in such a case I'd love to get a copy of that database by mail, if it's fine for you).

Comment 16

[Marco Bonardo [:mak]]

Attachment: patch v1.0 (checked-in)

This is the first attempt, can't be 100% sure this will fix the crash, but it's a start and we want these changes regardless.
- avoid passing the struct by value where not needed
- don't pass out the object in the constructor to avoid possible bad addref/release that could delete the object

To check for it to be fixed I'll make a trybuild for Stefan, then I'll keep checking crash-stats.

Comment 17

[Shawn Wilsher :sdwilsh]

Comment on attachment 509858 [details] [diff] [review]
patch v1.0 (checked-in)

r=sdwilsh

Comment 18

[Marco Bonardo [:mak]]

Stefan, could you try this build please (the macosx64 one) http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-4daeecf9928a/

Comment 19

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Marco, macosx64-debug crashes right after starting. macosx64 starts but fails in the same way when deleting the history item.

https://crash-stats.mozilla.com/report/index/bp-b2d3b323-b002-4309-8c9e-2dc772110204

Comment 20

[Shawn Wilsher :sdwilsh]

Stefan - can you link to one of the crashes you got with a normal nightly too please?  Want to make sure the crash is identical here.

Comment 21

[Marco Bonardo [:mak]]

(In reply to comment #19)
> Marco, macosx64-debug crashes right after starting.

hum, that is strange since it passed all tests on mac :( tomorrow I'll install it on my Mac and check.

Shawn, comment 10 has a crash report from Stefan

Comment 22

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I don't know if this means anything, but I compiled mozilla from trunk and it fails on my profile with the following:

*** Compartment mismatch 0x107027600 vs. 0x12915f200
Assertion failure: compartment mismatched, at /Users/stefan/Mozilla/mozilla/js/src/jscntxtinlines.h:541

(I'll try to build b10 - My goal was to run it in GDB to get more info about the crash that this bug is about)

Comment 23

[Shawn Wilsher :sdwilsh]

(In reply to comment #21)
> hum, that is strange since it passed all tests on mac :( tomorrow I'll install
> it on my Mac and check.
I don't think debug builds work unless you have the right packages installed locally, which most people won't have.

> Shawn, comment 10 has a crash report from Stefan
Whoops!  Not sure how I missed that...but yeah, it's the same crash still :/

Comment 24

[Marco Bonardo [:mak]]

(In reply to comment #23)
> (In reply to comment #21)
> > hum, that is strange since it passed all tests on mac :( tomorrow I'll install
> > it on my Mac and check.
> I don't think debug builds work unless you have the right packages installed
> locally, which most people won't have.

there was an opt in my trybuilds too.

btw, this build works pretty well on my Mac, since the changes are correct, even if they wouldn't fix the crash I'll push them.

Now, the compartment mismatch thing is expected to be fatal or to assert, but doesn't look related to this bug (that is cpp), I'd suggest to fie a bug aparts in core/Javascript engine if you have consistent str to reproduce it.

keeping investigating.

Comment 25

[Marco Bonardo [:mak]]

Comment on attachment 509858 [details] [diff] [review]
patch v1.0 (checked-in)

http://hg.mozilla.org/mozilla-central/rev/7cb3e9795d04

leaving the bug open to keep on the investigation.

Comment 26

[Marco Bonardo [:mak]]

The only reported crash (so far) after the fix
https://crash-stats.mozilla.com/report/index/cb785485-117e-4646-a757-858f82110207
the stack is slightly different, the crash point is the same.

Comment 27

[Marco Bonardo [:mak]]

6 crashes reported in b12pre

https://crash-stats.mozilla.com/report/list?product=Firefox&range_value=2&range_unit=weeks&&signature=AsyncGetBookmarksForURI%3Cvoid%20%28nsNavBookmarks%3A%3A*%29%28const%20mozilla%3A%3Aplaces%3A%3AItemChangeData%26%29%2C%20mozilla%3A%3Aplaces%3A%3AItemChangeData%3E%3A%3AHandleResult

Comment 28

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Yeah I'm still seeing this in b12 when deleting random history items. Let me know if I can do additional testing or debugging. I have a profile that has this behaviour.

Comment 29

[Marco Bonardo [:mak]]

I will post a new build shortly (working on related bug 637679)

Comment 30

[Marco Bonardo [:mak]]

could you please try to reproduce in this build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-91a962414422/

Comment 31

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Ok, the good news:

I could reproduce a crash triggered by deleting a specific history item. With this trybuild that crash does not happen anymore.

The bad news:

It is a different bug. The crashes that I see for this history item are happening in ToNewUnicode().

See https://crash-stats.mozilla.com/report/index/bp-93c69ba0-f3c5-4bff-8f46-552132110301

Comment 32

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Oh looking at the stack trace of

https://crash-stats.mozilla.com/report/index/bp-93c69ba0-f3c5-4bff-8f46-552132110301

it seems that we actually mostly get through

AsyncGetBookmarksForURI<...>::HandleResult

but then crash in ToNewUnicode(), which is reported in

https://bugzilla.mozilla.org/show_bug.cgi?id=634796

Comment 33

[Marco Bonardo [:mak]]

I don't understand, did you crashed in ToNewUnicode with the build in comment 30? The build id doesn't seem the one from the try build.

Comment 34

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I will test again and make sure I am less confusing this time.

Comment 35

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Ok, tested again:

When running 4.0b12: I have a profile where deleting history items crashes the browser. However, it crashes PAST the top call in the stack trace for this bug (AsyncGetBookmarksForURI). See bug 634796, which is highly likely what I see.

When running your build, all is fine.

So I think this means that b12 actually had a fix for this bug but then triggered the next one (634796) which is fixed in the build that you gave me.

Comment 36

[Marco Bonardo [:mak]]

I assume this is fixed by bug 637957, since it involves the same code path and has same symptoms.

Yeah, b12 had a change in the behavior that changed the stack.