Closed Bug 892340 Opened 12 years ago Closed 12 years ago

Remove Off-Main-Thread XPCWrappedJS refcounting from (UDP)ServerSocketListenerProxy

Categories

(Core :: Networking, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla25
Tracking Status
firefox23 --- unaffected
firefox24 --- verified
firefox25 --- verified

People

(Reporter: hectorz, Assigned: hectorz)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #850246 +++ Bug 870916 comment 23: > (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.
Report link, bp-909944d6-c958-44af-b8fe-411de2130710, and stack trace: Frame Module Signature Source 0 xul.dll nsXPTCStubBase::AddRef() xpcom/reflect/xptcall/src/xptcall.cpp 1 xul.dll nsRefPtr<mozilla::dom::CDATASection>::nsRefPtr<mozilla::dom::CDATASection>(mozilla::dom::CDATASection *) obj-firefox/dist/include/nsAutoPtr.h 2 xul.dll `anonymous namespace'::ServerSocketListenerProxy::OnPacketReceivedRunnable::OnPacketReceivedRunnable(nsIUDPServerSocketListener *,nsIUDPServerSocket *,nsIUDPMessage *) netwerk/base/src/nsUDPServerSocket.cpp 3 xul.dll `anonymous namespace'::ServerSocketListenerProxy::OnPacketReceived(nsIUDPServerSocket *,nsIUDPMessage *) netwerk/base/src/nsUDPServerSocket.cpp 4 xul.dll nsUDPServerSocket::OnSocketReady(PRFileDesc *,short) netwerk/base/src/nsUDPServerSocket.cpp 5 xul.dll nsSocketTransportService::DoPollIteration(bool) netwerk/base/src/nsSocketTransportService2.cpp 6 xul.dll nsSocketTransportService::Run() netwerk/base/src/nsSocketTransportService2.cpp 7 xul.dll nsThread::ProcessNextEvent(bool,bool *) xpcom/threads/nsThread.cpp 8 xul.dll nsThread::ThreadFunc(void *) xpcom/threads/nsThread.cpp 9 nss3.dll _PR_NativeRunThread nsprpub/pr/src/threads/combined/pruthr.c 10 nss3.dll pr_root nsprpub/pr/src/md/windows/w95thred.c 11 msvcr100.dll _callthreadstartex f:\\dd\\vctools\\crt_bld\\self_x86\\crt\\src\\threadex.c 12 msvcr100.dll _threadstartex f:\\dd\\vctools\\crt_bld\\self_x86\\crt\\src\\threadex.c 13 kernel32.dll BaseThreadInitThunk 14 ntdll.dll __RtlUserThreadStart 15 ntdll.dll _RtlUserThreadStart
Blocks: 770840
Severity: normal → critical
Keywords: crash, regression
Attached patch PatchSplinter Review
I've tested my local build, nsIUDPServerSocket works w/o crash
Attachment #773802 - Flags: review?(mcmanus)
Crash Signature: [@ nsXPTCStubBase::AddRef()] → [@ nsXPTCStubBase::AddRef() ]
Attachment #773802 - Flags: review?(mcmanus) → review+
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Comment on attachment 773802 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 770840 User impact if declined: Firefox will crash when nsIUDPServerSocket receive any packet Testing completed (on m-c, etc.): Same patch for nsIServerSocket landed in bug 850246 Risk to taking this patch (and alternatives if risky): Low risk, see above String or IDL/UUID changes made by this patch: None
Attachment #773802 - Flags: approval-mozilla-aurora?
Comment on attachment 773802 [details] [diff] [review] Patch low risk patch fixing crash regression.
Attachment #773802 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: checkin-needed
Is there a test to verify this is fixed?
Flags: needinfo?(bzhao)
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #8) > Is there a test to verify this is fixed? To manually verify the fix: Run the following code in browser console: > let udpServer = { > _server: null, > init: function() { > this._server = Cc["@mozilla.org/network/server-socket-udp;1"] > .createInstance(Ci.nsIUDPServerSocket); > this._server.init(49941, false); > this._server.asyncListen(this); > }, > uninit: function() { > this._server.close(); > }, > onStopListening: function(aServ, aStatus) {}, > onPacketReceived: function(aServ, aMessage) { > console.log(aMessage.data); > } > }; > > udpServer.init(); Then send a udp packet to it, e.g. in the terminal > nc -u 127.0.0.1 49941 Type some text in the prompt and then hit Enter Before the fix Firefox will crash, after the fix the text send using nc should appear in the browser console. I hope this is helpful. FYI, the code snippet above (using nsIUDPServerSocket) will stop working once bug 869869 is landed, IIUC.
Flags: needinfo?(bzhao)
Thanks Hector, flagging for verification with the steps in comment 9.
Keywords: verifyme
Verified as fixed on Firefox 24 RC (20130910201120). I verified this on Ubuntu 13.04 32bit and Mac OS X 10.7.5 64bit. I couldn't verify it on my Windows 7 because no netcat I got worked on it.
QA Contact: ioana.budnar
Also verified on Firefox 25 beta 1, on Ubuntu 13.04 32bit and Mac OSX 10.8.4 64bit.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: