Closed Bug 870916 Opened 7 years ago Closed 7 years ago
crash in ns
Socket Transport::Build Socket @ ns XPTCStub Base::Add Ref
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: 18.104.22.16832 Has dual GPUs. GPU #2: AdapterVendorID2: 0x10de, AdapterDeviceID2: 0x1050, AdapterSubsysID2: 16821043, AdapterDriverVersion2: 22.214.171.1244D2D? 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
Crash Signature: [@ nsXPTCStubBase::AddRef()] → [@ nsXPTCStubBase::AddRef()] [@ nsXPCWrappedJS::AddRef()]
Hardware: x86 → All
If I'm right, we probably need to change http://mxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsSocketTransport2.cpp#1012 and http://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/src/TransportSecurityInfo.h#87.
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).
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
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.
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.
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+
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 :)
You need to log in before you can comment on or make changes to this bug.