849585 - crash in nsSocketTransport::SendStatus from refcount error or nsSocketTransport being freed and/or overwritten

Status: RESOLVED WORKSFORME

(MailNews Core :: Networking, defect)

Description

[Wayne Mery (:wsmwk)]

nsCOMPtr_base::assign_with_AddRef(nsISupports*) | nsSocketTransport::SendStatus(tag_nsresult)

I crashed bp-a4ee16e9-67ca-49ec-9fc7-40d942130301 

============================================================= 
0	xul.dll	nsCOMPtr_base::assign_with_AddRef	objdir-tb/mozilla/xpcom/build/nsCOMPtr.cpp:48
1	xul.dll	nsSocketTransport::SendStatus	netwerk/base/src/nsSocketTransport2.cpp:873
2	xul.dll	nsSocketTransport::OnSocketConnected	netwerk/base/src/nsSocketTransport2.cpp:1408
3	xul.dll	nsSocketTransport::OnSocketReady	netwerk/base/src/nsSocketTransport2.cpp:1576
4	xul.dll	nsSocketTransportService::DoPollIteration	netwerk/base/src/nsSocketTransportService2.cpp:785
5	xul.dll	nsSocketTransportService::Run	netwerk/base/src/nsSocketTransportService2.cpp:642 

hg@1 865  nsSocketTransport::SendStatus(nsresult status)
hg@1 866  {
dwitte@56649 867 SOCKET_LOG(("nsSocketTransport::SendStatus [this=%x status=%x]\n", this, status));
hg@1         868 
hg@1         869 nsCOMPtr<nsITransportEventSink> sink;
ehsan@102997 870 uint64_t progress;
hg@1         871 {
jones@64576  872 MutexAutoLock lock(mLock);
hg@1         873 sink = mEventSink; 

Nothing in crash stats prior to TB18.0a1 bp-607f511f-4536-4e61-8821-7ea8e2120925
18.0a2 bp-1c6abdb8-0644-4623-a928-456fc2121030
18.0b1 bp-c9733e16-6a07-4a4d-8f87-5aeed2121202
19.0b1 bp-addf9ef5-7590-4ee8-b419-5767e2130125

Comment 1

[:aceman]

Does the 'sink = mEventSink' line need some nsCOMPtr glue? E.g. NS_IF_ADDREF or something?

Comment 2

[Wayne Mery (:wsmwk)]

(In reply to :aceman from comment #1)
> Does the 'sink = mEventSink' line need some nsCOMPtr glue? E.g. NS_IF_ADDREF
> or something?

good question :)

still occurs in current versions. Same line for 3 different stacks

bp-86f6c315-0aa9-4d4e-aedd-360d12140909
 0 	xul.dll	nsCOMPtr_base::assign_with_AddRef(nsISupports*)	xpcom/glue/nsCOMPtr.cpp
1 	xul.dll	nsSocketTransport::SendStatus(tag_nsresult)	netwerk/base/src/nsSocketTransport2.cpp
2 	xul.dll	nsSocketInputStream::Read(char*, unsigned int, unsigned int*)	netwerk/base/src/nsSocketTransport2.cpp
3 	xul.dll	nsStreamCopierIB::ConsumeInputBuffer(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*)	xpcom/io/nsStreamUtils.cpp 

bp-f6427afc-d7c0-4315-bfc5-396e52140906
 0 	xul.dll	nsCOMPtr_base::assign_with_AddRef(nsISupports*)	xpcom/glue/nsCOMPtr.cpp
1 	xul.dll	nsSocketTransport::SendStatus(tag_nsresult)	netwerk/base/src/nsSocketTransport2.cpp
2 	xul.dll	nsSocketTransport::OnSocketConnected()	netwerk/base/src/nsSocketTransport2.cpp
3 	xul.dll	nsSocketTransport::OnSocketReady(PRFileDesc*, short)	netwerk/base/src/nsSocketTransport2.cpp
4 	xul.dll	nsSocketTransportService::DoPollIteration(bool)	netwerk/base/src/nsSocketTransportService2.cpp 

bp-d923d8f9-9477-4240-b69f-7666b2140819
 0 	xul.dll	nsCOMPtr_base::assign_with_AddRef(nsISupports*)	objdir-tb/mozilla/xpcom/build/nsCOMPtr.cpp
1 	xul.dll	nsSocketTransport::SendStatus(tag_nsresult)	netwerk/base/src/nsSocketTransport2.cpp
2 	xul.dll	nsSocketTransport::ResolveHost()	netwerk/base/src/nsSocketTransport2.cpp
3 	xul.dll	nsSocketTransport::OnSocketEvent(unsigned int, tag_nsresult, nsISupports*)	netwerk/base/src/nsSocketTransport2.cpp

Comment 3

[:Irving Reid (No longer working on Firefox)]

(In reply to Wayne Mery (:wsmwk) from comment #2)
> (In reply to :aceman from comment #1)
> > Does the 'sink = mEventSink' line need some nsCOMPtr glue? E.g. NS_IF_ADDREF
> > or something?
> 
> good question :)

No, both sides of the assignment are already nsCOMPtr<...>, so the reference counting should be fine. My suspicion is that another thread has thrown away the object pointed to by mEventSink due to a reference counting problem elsewhere, and the failure is intermittent because it's relatively rare that the memory gets completely unmapped when the object is freed.

Another possibility is that the nsSocketTransport itself is being freed and/or overwritten, but I'd expect that to crash in the previous line (trying to grab a lock on the mLock field).

Comment 4

[Ludovic Hirlimann [:Usul]]

Removing myslef on all the bugs I'm cced on. Please NI me if you need something on MailNews Core bugs from me.

Comment 5

[Wayne Mery (:wsmwk)]

No version 68 crashes and only 17 thunderbird crashes in the past 3 months, mostly version 52 [1]. Perhaps we can claim victory at least in part from the many refcount bugs fixed by Jorg and Magnus in the past two years https://mzl.la/2poSTb8. (or the signature morphed)

[1] https://crash-stats.mozilla.org/signature/?signature=nsCOMPtr_base%3A%3Aassign_with_AddRef%20%7C%20mozilla%3A%3Anet%3A%3AnsSocketTransport%3A%3ASendStatus&date=%3E%3D2019-08-17T03%3A08%3A00.000Z&date=%3C2019-11-17T03%3A08%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_columns=startup_crash&_sort=-date&page=1#reports