1373064 - Crash in nsTHashtable<T>::GetEntry | nsClassHashtable<T>::Get | mozilla::net::nsHttpTransaction::ShouldStopReading

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Mats Palmgren (inactive)]

This bug was filed from the Socorro interface and is 
report bp-55094a05-4bf5-4ef1-b7ba-a386d0170614.
=============================================================

Looks like a recent regression, first reported crash is for 20170611030208.
Possibly a regression from bug 1369496?

nsTHashtable<mozilla::dom::WebIDLNameTableEntry>::GetEntry(mozilla::dom::WebIDLNameTableKey const&)
nsClassHashtable<nsUint64HashKey, nsTArray<RefPtr<mozilla::net::nsHttpConnectionMgr::PendingTransactionInfo> > >::Get(unsigned __int64 const&)
mozilla::net::nsHttpTransaction::ShouldStopReading()
mozilla::net::nsHttpTransaction::WriteSegments(mozilla::net::nsAHttpSegmentWriter*, unsigned int, unsigned int*)
mozilla::net::nsHttpConnection::OnSocketReadable()
mozilla::net::nsHttpConnection::OnInputStreamReady(nsIAsyncInputStream*)
mozilla::net::nsSocketInputStream::OnSocketReady(nsresult)
mozilla::net::nsSocketTransport::OnSocketDetached(PRFileDesc*)
mozilla::net::nsSocketTransportService::DetachSocket(mozilla::net::nsSocketTransportService::SocketContext*, mozilla::net::nsSocketTransportService::SocketContext*)
mozilla::net::nsSocketTransportService::DoPollIteration(mozilla::BaseTimeDuration<mozilla::TimeDurationValueCalculator>*)
mozilla::net::nsSocketTransportService::Run()
[...]

Comment 1

[Honza Bambas (:mayhemer)]

Looks like we are missing some null checks.

Comment 2

[Honza Bambas (:mayhemer)]

Attachment: v1 (add a nullcheck)

When can this happen?  Can there be no entry when there is a transaction with the key?  Is it something with coalescing?

Comment 3

[Patrick McManus [:mcmanus]]

Comment on attachment 8878044 [details] [diff] [review]
v1 (add a nullcheck)

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

if this is before the getorcreateentry() call this could happen.. also entries can be removed from ct if they go idle.

Comment 4

[Honza Bambas (:mayhemer)]

Comment on attachment 8878044 [details] [diff] [review]
v1 (add a nullcheck)

Approval Request Comment
[Feature/Bug causing the regression]: bug 1369496
[User impact if declined]: crash (null crash)
[Is this code covered by automated tests?]: yes, but not for the crashing case apparently...
[Has the fix been verified in Nightly?]: not even landed
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: instead of null-deref we early return false, the caller is safe with it
[String changes made/needed]: none

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf586f9f5eeb
Add null-check for a connection entry to nsHttpConnectionMgr::IsConnEntryUnderPressure. r=mcmanus

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bf586f9f5eeb

Comment 7

[Julien Cristau [:jcristau]]

Comment on attachment 8878044 [details] [diff] [review]
v1 (add a nullcheck)

add null check to fix crash, beta55+

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e2638104f016