563573 - Crash [@ XPCJSStackFrame::GetFilename(char**) ][@ nsRevocableEventPtr<nsRunnableMethod<nsMediaChannelStream, void, int> >::Revoke() ]

Status: VERIFIED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[IU]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100503 Minefield/3.7a5pre (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100503 Minefield/3.7a5pre (.NET CLR 3.5.30729) ID:20100503040502

When visiting the link "Jay posted a request," from the above linked URL (http://www.scripting.com/stories/2010/05/03/rbtn50.html), a crash with either of the following signatures is received:

[@ XPCJSStackFrame::GetFilename(char**) ]
http://crash-stats.mozilla.com/report/index/bp-6bbdb9b0-0758-4423-9fb8-edd712100503

[@ nsRevocableEventPtr<nsRunnableMethod<nsMediaChannelStream, void, int> >::Revoke() ]
http://crash-stats.mozilla.com/report/index/bp-576a19e7-1852-4f7e-a4cf-249f72100503

Happens in safe mode too.

Attaching WinDbg log...

Here are the respective stacks:

[@ XPCJSStackFrame::GetFilename(char**) ]
-----------------------------------------------------------
0  	xul.dll  	XPCJSStackFrame::GetFilename  	 js/src/xpconnect/src/xpcstack.cpp:280
1 	xul.dll 	nsRevocableEventPtr<nsRunnableMethod<nsMediaChannelStream,void,0> >::Revoke 	obj-firefox/dist/include/nsThreadUtils.h:445
2 	xul.dll 	nsHtml5TreeOpExecutor::WillBuildModel 	parser/html/nsHtml5TreeOpExecutor.h:146
3 	xul.dll 	nsHtml5StreamParser::OnStartRequest 	parser/html/nsHtml5StreamParser.cpp:559
4 	xul.dll 	nsDocumentOpenInfo::OnStartRequest 	uriloader/base/nsURILoader.cpp:290
5 	xul.dll 	nsHttpChannel::CallOnStartRequest 	netwerk/protocol/http/src/nsHttpChannel.cpp:824
6 	xul.dll 	nsHttpChannel::ProcessNormal 	netwerk/protocol/http/src/nsHttpChannel.cpp:1130
7 	xul.dll 	nsHttpChannel::ProcessResponse 	netwerk/protocol/http/src/nsHttpChannel.cpp:999
8 	xul.dll 	nsHttpChannel::OnStartRequest 	netwerk/protocol/http/src/nsHttpChannel.cpp:5183
9 	xul.dll 	nsInputStreamPump::OnStateStart 	netwerk/base/src/nsInputStreamPump.cpp:441
10 	xul.dll 	nsInputStreamPump::OnInputStreamReady 	netwerk/base/src/nsInputStreamPump.cpp:397
11 	xul.dll 	nsInputStreamReadyEvent::Run 	xpcom/io/nsStreamUtils.cpp:191
12 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:527
13 	xul.dll 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:142
14 	xul.dll 	xul.dll@0x9b443b 	
15 	xul.dll 	MessageLoop::RunInternal 	ipc/chromium/src/base/message_loop.cc:216
16 	xul.dll 	MessageLoop::RunHandler 	ipc/chromium/src/base/message_loop.cc:199
17 	xul.dll 	xul.dll@0x30b3a3 	
18 	xul.dll 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:173
19 	nspr4.dll 	nspr4.dll@0x187ff 	
20 	xul.dll 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:175
21 	xul.dll 	nsAppShell::Run 	widget/src/windows/nsAppShell.cpp:239



[@ nsRevocableEventPtr<nsRunnableMethod<nsMediaChannelStream, void, int> >::Revoke() ] 
---------------------------------------------------------------------------
0  	xul.dll  	nsRevocableEventPtr<nsRunnableMethod<nsMediaChannelStream,void,0> >::Revoke  	 obj-firefox/dist/include/nsThreadUtils.h:445
1 	xul.dll 	nsContentSink::WillBuildModelImpl 	
2 	xul.dll 	nsDocumentOpenInfo::OnStartRequest 	uriloader/base/nsURILoader.cpp:290
3 	xul.dll 	nsHttpChannel::CallOnStartRequest 	netwerk/protocol/http/src/nsHttpChannel.cpp:824
4 	xul.dll 	nsHttpChannel::ProcessNormal 	netwerk/protocol/http/src/nsHttpChannel.cpp:1130
5 	xul.dll 	nsHttpChannel::ProcessResponse 	netwerk/protocol/http/src/nsHttpChannel.cpp:999
6 	xul.dll 	nsHttpChannel::OnStartRequest 	netwerk/protocol/http/src/nsHttpChannel.cpp:5183
7 	xul.dll 	nsInputStreamPump::OnStateStart 	netwerk/base/src/nsInputStreamPump.cpp:441
8 	xul.dll 	nsInputStreamPump::OnInputStreamReady 	netwerk/base/src/nsInputStreamPump.cpp:397
9 	xul.dll 	nsInputStreamReadyEvent::Run 	xpcom/io/nsStreamUtils.cpp:191
10 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:527
11 	xul.dll 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:142
12 	xul.dll 	xul.dll@0x9b443b 	
13 	xul.dll 	MessageLoop::RunInternal 	ipc/chromium/src/base/message_loop.cc:216
14 	xul.dll 	MessageLoop::RunHandler 	ipc/chromium/src/base/message_loop.cc:199
15 	xul.dll 	xul.dll@0x30b3a3 	
16 	xul.dll 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:173
17 	nspr4.dll 	nspr4.dll@0x187ff 	
18 	xul.dll 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:175
19 	xul.dll 	nsAppShell::Run 	widget/src/windows/nsAppShell.cpp:239


Reproducible: Always

Steps to Reproduce:
1. Make sure your browser is not blocking content (maybe use a new profile)
2. Visit the linked URL
3. Click the "Jay posted a request" link
4. Crash

Comment 1

[IU]

Attachment: WinDBG log of crash

Comment 2

[alex_mayorga]

I also get the [@ nsRevocableEventPtr<nsRunnableMethod<nsMediaChannelStream, void, int> >::Revoke() ] crash but also this other two crashes with different signatures:
http://crash-stats.mozilla.com/report/index/bp-f96f6439-645f-415a-ab48-5b55b2100503
http://crash-stats.mozilla.com/report/index/bp-0e764982-ce2c-4ce4-afc4-867d22100503

Comment 3

[Alice0775 White]

Regression window:

Works:
http://hg.mozilla.org/mozilla-central/rev/861f07be1937
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100426 Minefield/3.7a5pre ID:20100426155632

Fails:
http://hg.mozilla.org/mozilla-central/rev/fa53318b87aa
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100426 Minefield/3.7a5pre ID:20100426171532

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=861f07be1937&tochange=fa53318b87aa

Comment 4

[Brian Carpenter [:geeknik]]

I'm seeing this with nightly build 20100506040636. Win7 Pro x64, HTML5=on, OOPP=on, D2D=on, DW=on.

Comment 5

[Ben Newman (:bnewman) (:benjamn)]

nsRevocableEventPtr is pretty clearly at risk of retaining a dangling pointer when T is not a smart-pointer type.  I'm trying to figure out why this didn't happen before...

Comment 6

[Ben Newman (:bnewman) (:benjamn)]

On second thought, there really is no reason for nsRevocableEventPointer to exist apart from nsRefPtr.  It's a templatized class, so unused methods are not compiled.  That means the presence of T::Revoke is not even enforced (the whole reason for nsRevocableEventPointer) unless nsRevocableEventPointer<T>::Revoke is ever called.  But calling Revoke against an nsRefPtr<T> forces the compiler to check that T::Revoke is supported, anyway, so nsRefPtr<T> provides all the benefits (and none of the problems) of nsRevocableEventPointer<T>.

Comment 7

[Ben Newman (:bnewman) (:benjamn)]

(In reply to comment #6)

nsRevocableEventP[ointe][t]r

By the way, everyone should use this edit notation:
http://www.azarask.in/blog/post/collaboration_made_simple_with_bracket_notation/

Comment 8

[Ben Newman (:bnewman) (:benjamn)]

Thought experiment underway:
http://hg.mozilla.org/try/rev/e6341236a376

Comment 9

[Ben Newman (:bnewman) (:benjamn)]

(In reply to comment #6)

On third thought, nsRevocableEventPtr does provide a null-safe version of Revoke, and calls Revoke on operator=.  Replacing it with nsRefPtr is more trouble than necessary.

Instead, I propose making nsRevocableEventPtr<T>::mEvent an nsRefPtr<T> instead of a T*.  That alone should prevent this crash.

Comment 10

[Ben Newman (:bnewman) (:benjamn)]

Attachment: Patch to fix.

This patch makes nsRevocableEventPtr refcount its event object.  Note that this works even with non-owning nsRunnableMethods, because the event itself is an nsRunnable supporting AddRef/Release.  The refcountability of the receiving object of the runnable method doesn't matter in this context.

Comment 11

[Ben Newman (:bnewman) (:benjamn)]

Comment on attachment 443973 [details] [diff] [review]
Patch to fix.

I was able to reproduce the crash on linux, so I imagine it's a general problem.  The patch prevents it from happening on linux, so with any luck the solution is equally general :)

r? to you, bent, since I know you love anything nsRunnableMethod-related.

Comment 12

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 443973 [details] [diff] [review]
Patch to fix.

Looks good.

Comment 13

[Ben Newman (:bnewman) (:benjamn)]

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/229473c36099

Comment 14

[alex_mayorga]

The STR no longer crashes Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a5pre) Gecko/20100514 Minefield/3.7a5pre ID:20100514040039

Comment 15

[IU]

Verified fixed:
http://hg.mozilla.org/mozilla-central/rev/932c6dc1932b
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100515 Minefield/3.7a5pre ID:20100515040001

Comment 16

[Ben Newman (:bnewman) (:benjamn)]

Thanks for bringing this to my attention, and thanks for the verifications.