Closed Bug 870916 Opened 7 years ago Closed 7 years ago

crash in nsSocketTransport::BuildSocket @ nsXPTCStubBase::AddRef

Categories

(Core :: Networking, defect, critical)

23 Branch
All
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla24
Tracking Status
firefox22 --- unaffected
firefox23 --- affected

People

(Reporter: scoobidiver, Assigned: jdm)

References

Details

(Keywords: crash, regression, Whiteboard: [startupcrash])

Crash Data

Attachments

(1 file, 1 obsolete file)

It first showed up in 23.0a1/20130510. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ea059733677c&tochange=08be63954b6b
It might be a regression from bug 870579.

Signature 	nsXPTCStubBase::AddRef() More Reports Search
UUID	75fca066-dec0-4052-bfd1-f827c2130510
Date Processed	2013-05-10 17:55:07
Uptime	4
Last Crash	6 seconds before submission
Install Age	17 seconds since version was first installed.
Install Time	2013-05-10 17:54:27
Product	Firefox
Version	23.0a1
Build ID	20130510041606
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7601 Service Pack 1
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 42 stepping 7
Crash Reason	EXCEPTION_BREAKPOINT
Crash Address	0x5922f12f
App Notes 	
AdapterVendorID: 0x8086, AdapterDeviceID: 0x0116, AdapterSubsysID: 16821043, AdapterDriverVersion: 9.17.10.2932
Has dual GPUs. GPU #2: AdapterVendorID2: 0x10de, AdapterDeviceID2: 0x1050, AdapterSubsysID2: 16821043, AdapterDriverVersion2: 9.18.13.1144D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ 
Processor Notes 	sp-processor05_phx1_mozilla_com_3440:2012
EMCheckCompatibility	True
Adapter Vendor ID	0x8086
Adapter Device ID	0x0116
Total Virtual Memory	4294836224
Available Virtual Memory	3823108096
System Memory Use Percentage	42
Available Page File	12738912256
Available Physical Memory	4845830144

Frame 	Module 	Signature 	Source
0 	xul.dll 	nsXPTCStubBase::AddRef 	xpcom/reflect/xptcall/src/xptcall.cpp:30
1 	xul.dll 	nsCOMPtr_base::assign_with_AddRef 	obj-firefox/xpcom/build/nsCOMPtr.cpp:48
2 	msvcr100.dll 	zzz_AsmCodeRange_End 	
3 	xul.dll 	nsSocketTransport::BuildSocket 	netwerk/base/src/nsSocketTransport2.cpp:1016
4 	xul.dll 	nsSocketTransport::InitiateSocket 	netwerk/base/src/nsSocketTransport2.cpp:1094
5 	xul.dll 	nsSocketTransport::OnSocketEvent 	netwerk/base/src/nsSocketTransport2.cpp:1518
6 	xul.dll 	nsSocketEvent::Run 	netwerk/base/src/nsSocketTransport2.cpp:66
7 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:627
8 	xul.dll 	NS_ProcessNextEvent 	obj-firefox/xpcom/build/nsThreadUtils.cpp:238
9 	xul.dll 	nsSocketTransportService::Run 	netwerk/base/src/nsSocketTransportService2.cpp:648
10 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:627
11 	xul.dll 	nsThread::ThreadFunc 	xpcom/threads/nsThread.cpp:265
12 	nss3.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:395
13 	nss3.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:90
14 	msvcr100.dll 	_callthreadstartex 	f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c:314
15 	msvcr100.dll 	_threadstartex 	f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c:292
16 	kernel32.dll 	BaseThreadInitThunk 	
17 	ntdll.dll 	__RtlUserThreadStart 	
18 	ntdll.dll 	_RtlUserThreadStart 	

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsXPTCStubBase%3A%3AAddRef%28%29
This is almost certainly triggered by bug 770840, meaning that bug 850252 wasn't enough.
Crash Signature: [@ nsXPTCStubBase::AddRef()] → [@ nsXPTCStubBase::AddRef()] [@ nsXPCWrappedJS::AddRef()]
Hardware: x86 → All
Blocks: 770840
Blocks: 773610
Does this look likely to you, Bobby?
re: comment 2: we also pass the callbacks into PSM, so presumably we also need to change these?

   http://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/src/nsNSSIOLayer.cpp#153

   http://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/src/nsNSSIOLayer.cpp#145  
   (I assume we'd want to assert main-thread only for gets?)
(In reply to Josh Matthews [:jdm] from comment #3)
> Does this look likely to you, Bobby?

Likely in what sense? I don't know this code at all.
Likely in that this is nsXPTCStubBase, not nsXPCWrappedJS.
(In reply to Josh Matthews [:jdm] from comment #6)
> Likely in that this is nsXPTCStubBase, not nsXPCWrappedJS.

Oh, sorry. They are one in the same.
(nsXPTCStubBase is the glue that allows XPCWrappedJS to masquerade as arbitrary XPCOM interface types).
Assignee: nobody → josh
Summary: crash in nsXPTCStubBase::AddRef → crash in nsSocketTransport::BuildSocket @ nsXPTCStubBase::AddRef
(In reply to Jason Duell (:jduell) from comment #4)
> re: comment 2: we also pass the callbacks into PSM, so presumably we also
> need to change these?
The stack trace on Mac is:
Frame 	Module 	Signature 	Source
0 	XUL 	nsXPCWrappedJS::AddRef 	js/xpconnect/src/XPCWrappedJS.cpp:155
1 	XUL 	nsCertVerificationJob::Run 	obj-firefox/x86_64/dist/include/nsCOMPtr.h:566
2 	XUL 	nsCertVerificationThread::Run 	security/manager/ssl/src/nsCertVerificationThread.cpp:142
3 	libnss3.dylib 	null_signal_handler 	
4 	libnss3.dylib 	_pt_root 	
5 	libsystem_c.dylib 	libsystem_c.dylib@0x147a2 	
6 	libsystem_c.dylib 	libsystem_c.dylib@0x11e1 	
7 	libnss3.dylib 	null_signal_handler

See also bug 870887 for Calendar.
Actually, I'm really confused about this abort. nsSocketTransport's mCallbacks is implicated, but all the callers of nsSocketTransport::SetSecurityCallbacks pass pointers to C++ types which should not trigger this. I can't figure out where nsSocketTransport would obtain a JS-implemented callbacks object.
AWOEI#U%(*#QAJFQO#@EEEEEEEEEKDWKMD

The user comments on the related crash reports tipped me off to the KeeFox addon, and I found this: https://github.com/luckyrat/KeeFox/blob/master/Firefox%20addon/KeeFox/modules/session.js#L109
Attachment #751163 - Flags: review?(bsmith)
Does this patch make what that addon does a non-issue? We could also just break the addon and make sure not to crash. Does this need to be IDL-exposed? If not, let's unexpose it. If so, let's make SetSecurityCallbacks check if its argument is a wrappedJS and throw if so. Easiest way to do this is probably to move IsWrappedJS from xpcprivate.h to xpcpublic.h and call it from necko.
No longer blocks: 770840
This should make what the addon does a non-issue and continue to work.
Comment on attachment 751163 [details] [diff] [review]
Make socket transport avoid off-main-thread usage of wrapped JS components.

https://tbpl.mozilla.org/?tree=Try&rev=12be5cb02e9d shows this isn't ready yet.
Attachment #751163 - Flags: review?(mcmanus)
Attachment #751163 - Flags: review?(bsmith)
I've kicked off another run with a much simpler patch: https://tbpl.mozilla.org/?tree=Try&rev=65ee375598c9
Attachment #751163 - Attachment is obsolete: true
Comment on attachment 753910 [details] [diff] [review]
Make socket transport avoid off-main-thread usage of wrapped JS components.

Try appears to like this one.
Attachment #753910 - Flags: review?(mcmanus)
Comment on attachment 753910 [details] [diff] [review]
Make socket transport avoid off-main-thread usage of wrapped JS components.

Review of attachment 753910 [details] [diff] [review]:
-----------------------------------------------------------------

ugh.
Attachment #753910 - Flags: review?(mcmanus) → review+
https://hg.mozilla.org/mozilla-central/rev/be5476e167e4
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
This crash still happen when I play with nsIUDPServerSocket, while nsIServerSocket works fine for me. Sample crash report: https://crash-stats.mozilla.com/report/index/909944d6-c958-44af-b8fe-411de2130710 , should I file a new bug?
(In reply to Hector Zhao [:hectorz] from comment #22)
> This crash still happen when I play with nsIUDPServerSocket, while
> nsIServerSocket works fine for me. Sample crash report:
> https://crash-stats.mozilla.com/report/index/909944d6-c958-44af-b8fe-
> 411de2130710 , should I file a new bug?

Seems like bug 850246 should also be done for /netwerk/base/src/nsUDPServerSocket.cpp, I'll give it a try.
Please do, but in a new bug :)
Depends on: 892340
You need to log in before you can comment on or make changes to this bug.