1019918 - http/2 push promise crash

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Patrick McManus [:mcmanus]]

1. PUSH_PROMISE sometimes crash Firefox

The test pages of push promise

https://http2.iijplus.jp/push/test1
https://http2.iijplus.jp/push/test2

often crash Firefox. Tesing the same pages on my internal test server
sometimes works fine but sometime crashes. I'm not sure what is
different.One of the crash reports is

https://crash-stats.mozilla.com/report/index/3991d290-5388-4659-bc15-1e1962140603

I hope it helps.

Comment 1

[u408661]

Attachment: patch

OK, so here's what seems to be happening. The server sends the PUSH_PROMISE, and then starts sending us data, which we are happily accepting, and storing in the SpdyPushCache. Then, for some reason (this I haven't diagnosed yet, it could be a bug on either end), the server sends a RST_STREAM on the pushed stream. So, we go into CleanupStream, where we remove the pushed stream from mPushedStreams, and its push buffer from mStreamTransactionHash. This causes both of these guys to be freed (the SpdyPushCache keeps a plain Http2Stream *, so no ref counting there). We never remove it from the push cache, though. Then, eventually, a pull stream tries to match itself with a pushed stream, and "succeeds", finding the (now freed) stream in the push cache. Then it tries to use that pushed stream, and *boom*.

Note that the server doesn't always send us a RST_STREAM, which is why this is an intermittent (though common) crash.

This patch ensures we clean up the stream from the SpdyPushCache when we go to clean it up from everywhere else. This makes push (at least with iij) not 100% reliable, but at least it prevents crashing. Figuring out why we are sent a RST_STREAM will likely take more work (perhaps together in NYC if you guys have time) to debug.

Comment 2

[Patrick McManus [:mcmanus]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/d717efc44fd3

Comment 3

[Ryan VanderMeulen [:RyanVM]]

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