386614 - Crash [@ nsSocketTransportService::Shutdown] when toggling offline/online in 2 frames

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes current trunk builds after a few reloads.
The testcase uses enhanced privileges, so you need to download it to your computer to test.

It doesn't crash in 2006-05-10 build, but crashes in a 2006-05-12 build.
I think a regression from bug 326273, somehow.

https://crash-reports.mozilla.com/reports/report/index/ac234d0a-28a0-11dc-80c7-001a4bd43ed6
0  	nsSocketTransportService::Shutdown()
1 	nsIOService::SetOffline(int)
2 	NS_InvokeByIndex_P
3 	AutoJSSuspendRequest::SuspendRequest()
4 	XPC_NW_toString
5 	ReservedSlotIndexOK
6 	XPC_WN_GetterSetter(JSContext *,JSObject *,unsigned int,long *,long *)
7 	js_Invoke
8 	js_InternalInvoke
9 	js_InternalGetOrSet

Content of both iframes:
<script>
function toggleoffline(){
netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
var ioService = Components.classes["@mozilla.org/network/io-service;1"].
       getService(Components.interfaces.nsIIOService2);
ioService.offline = !ioService.offline;
}
setInterval(toggleoffline,20);
</script>

Comment 1

[Benjamin Smedberg]

There is a topcrasher in nsSocketTransportService::Shutdown, but it doesn't appear to be exactly the same as bp-ac234d0a-28a0-11dc-80c7-001a4bd43ed6 because it isn't reentrant. But marking blocking+ for the topcrasher.

biesi, can you take a look?

Comment 2

[Martijn Wargers (dead)]

I think I sent quite a few crash reports with this stacktrace. Maybe I caused this to be a topcrasher?

Comment 3

[timeless]

this doesn't seem very good:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/netwerk/base/src/nsIOService.cpp&rev=1.197&mark=646-647,650-651,653#643

but i don't think i can blame it....

i believe the problem is here:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/netwerk/base/src/nsSocketTransportService2.cpp&rev=1.25&mark=418,419#395
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/xpcom/threads/nsThread.cpp&rev=1.65&mark=442#405

I believe this means that Shutdown enables this code to reenter.

Comment 4

[timeless]

Attachment: maybe this works

Comment 5

[Martijn Wargers (dead)]

Comment on attachment 270834 [details] [diff] [review]
maybe this works

It works indeed, it makes the crash go away.
But I'm not really qualified to review this code.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 270834 [details] [diff] [review]
maybe this works

+        return NS_ERROR_FAILURE;

nsThread::Shutdown uses NS_ERROR_UNEXPECTED when it is reentered, perhaps that'd be better here as well?

Comment 7

[timeless]

Comment on attachment 270834 [details] [diff] [review]
maybe this works

mozilla/netwerk/base/src/nsSocketTransportService2.cpp		1.26

Comment 8

[timeless]

a real testcase should probably use nsITimer directly instead of setTimeout.

Comment 9

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071905 Minefield/3.0a7pre

Thanks for fixing!