1505834 - Crash in mozilla::net::nsHttpConnection::OnInputStreamReady

Status: RESOLVED DUPLICATE of bug 1451293

(Core :: Networking: HTTP, defect, P2)

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is
report bp-45c683d9-4dd0-4558-a691-e871c0181108.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll mozilla::net::nsHttpConnection::OnInputStreamReady netwerk/protocol/http/nsHttpConnection.cpp:2422
1 xul.dll void mozilla::net::SpdyConnectTransaction::CreateShimError netwerk/protocol/http/TunnelUtils.cpp:1322
2 xul.dll nsresult mozilla::net::SpdyConnectTransaction::WriteDataToBuffer netwerk/protocol/http/TunnelUtils.cpp:1346
3 xul.dll mozilla::net::SpdyConnectTransaction::WriteSegments netwerk/protocol/http/TunnelUtils.cpp:1376
4 xul.dll mozilla::net::Http2Stream::WriteSegments netwerk/protocol/http/Http2Stream.cpp:319
5 xul.dll mozilla::net::Http2Session::WriteSegmentsAgain netwerk/protocol/http/Http2Session.cpp:3387
6 xul.dll mozilla::net::nsHttpConnection::OnInputStreamReady netwerk/protocol/http/nsHttpConnection.cpp:2422
7 xul.dll void mozilla::net::nsSocketInputStream::OnSocketReady netwerk/base/nsSocketTransport2.cpp:293
8 xul.dll mozilla::net::nsSocketTransport::OnSocketReady netwerk/base/nsSocketTransport2.cpp:2317
9 xul.dll mozilla::net::nsSocketTransportService::Run netwerk/base/nsSocketTransportService2.cpp:1040

=============================================================

There is 1 crash in nightly 65 with buildid 20181108100100. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1434137.

[1] https://hg.mozilla.org/mozilla-central/rev?node=31c304e56e00

Comment 1

[u408661]

Hrm, this one is a bit trickier than the previous fallout crash from h2ws, but it does appear at first glance to be related to my refactoring for the websockets. Not sure yet how I changed the semantics of the code (since this isn't in websocket-related code, it's in code that should be the same as in previous versions, just a little refactored - obviously I messed that up somewhere along the line).

Taking a deeper look to see how I may have subtly changed expectations.

Comment 2

[u408661]

So after taking a look at this pretty thoroughly yesterday, I'm no longer convinced that this is related to bug 1434137. I can't find any change in semantics for the code leading up to this point, plus this crash happens mid-method, on a line that calls another method on the same object: https://hg.mozilla.org/mozilla-central/annotate/b3da3f53f8042d6e2e8f90cd0086e354d96ba2fc/netwerk/protocol/http/nsHttpConnection.cpp#l2422. Very strange. This happens after other members of the object have already been accessed in that method, so it's possible that the object is being freed while the method is running. It wouldn't be the first time we've seen this kind of behaviour with nsHttpConnection (see https://bugzilla.mozilla.org/show_bug.cgi?id=1451293)

There are other crashes with the same signature (https://crash-stats.mozilla.com/report/index/784379b4-534c-4e9d-bb1f-c54d30181106 is an example) which happen on exactly the same line, but don't have the tunnel code on the stack.

However, I would like to get a second pair of eyes on this before claiming my code refactoring isn't at fault - it's certainly possible that I missed something by being too close to what's going on.

Honza, you were the one who commented in 1451293 about improper ref-counting of nsHttpTransaction, could you also take a look, and let me know if you see something I missed? Thanks.

Comment 3

[u408661]

(In reply to Nicholas Hurley [:nwgh][:hurley] (he/him) from comment #2)
> Honza, you were the one who commented in 1451293 about improper ref-counting
> of nsHttpTransaction

Whoops, that's supposed to be nsHttpConnection.

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Note that these crashes go a LONG ways back (many in thunderbird), at least to 51 and probably 45 (view 6 months of crashes).  Also most of these have security signatures - wildptrs, exec of wildptrs.  Some (but not all) are marked as being from LOG() source lines, also OnSocketReady().

Not likely to be a recent rewrite unless that made this more frequent.

Comment 5

[Honza Bambas (:mayhemer)]

I won't get to this soon enough, but this should be taken care of with a P1 treatment.  We are currently a bit short of developers in the networking team.

Putting back to the triage pool.

Comment 6

[Calixte Denizet (:calixte)]

There is no assignee and no activity for 7 days?.
:selenamarie, should we change the priority to P2?

Comment 7

[Selena Deckelmann :selenamarie :selena]

Kershaw -- could you have a look here?

Comment 8

[Kershaw Chang [:kershaw]]

I've landed the patch in bug 1451293 which avoids accessing http connection from main thread directly.
Let's see if that patch can also fix this crash.

Comment 9

[Honza Bambas (:mayhemer)]

The last crash on Nightly is [1] which doesn't have the fix for bug 1451293. The frequency is low, so I'd let it bake longer.

Thanks Karshaw!

[1] https://crash-stats.mozilla.com/report/index/17aec7d3-fa9a-4009-ad63-a65a80190109

Comment 10

[Selena Deckelmann :selenamarie :selena]

Can we recheck?

Comment 11

[Honza Bambas (:mayhemer)]

Confirming this crash is gone.

Comment 12

[David Lawrence [:dkl]]

Removing employee no longer with company from CC list of private bugs.