1021396 - duplicate spdy syn_stream can hang necko thread

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Patrick McManus [:mcmanus]]

I'm testing with spdyproxy (spdy/3) and have seen the necko thread hang a couple times. I have one log with a lot of my personal profile in it. I'm working on getting one with a fresh profile to share.

The root cause is that the session sees 2 syn_reply frames for the same stream id. This happens to be a CONNECT stream - I'm not sure if that's relevant.

the spdy spec says "If an endpoint receives multiple SYN_REPLY frames for the same active stream ID, it MUST issue a stream error (Section 2.4.2) with the error code STREAM_IN_USE."

since that's a stream error (which does not close the session) the headers need to be uncompressed. The code uncompresses it first and then checks to see if this is a double reply.

The bug is that the decompression code will get stuck in a loop if called to process stream headers twice, as it frees the buffer after the first time. (and it can't make progress with 0 bytes of buffer). So it never completes the uncompression and it doesn't get to check if this is a double open.

this applies to spdy/3 and spdy/3.1 but not http/2. It doesn't apply to http/2 for a few reasons - chiefly multiple headers are simply legal for http/2 so the same assumption isn't made and the decompressor can actually allocate buffer as needed in http/2.

I'm going to turn the spdy stream error into a session error. That's just a protocol violation.

Comment 1

[Patrick McManus [:mcmanus]]

actually, as much as I want to make it a session error - spdy seems to explicit say this is OK but has to return a stream error.

Comment 2

[Patrick McManus [:mcmanus]]

Attachment: log.bz2

nspr log showing double syn_reply for stream 0x67 on same session

line 582147 syn_reply 0x67
4 data frames for 0x67.. about 3kb of data
line 607557 syn_reply 0x67

Comment 3

[Patrick McManus [:mcmanus]]

Attachment: 0001-bug-1021396-duplicate-spdy-syn_stream-can-hang-necko.patch

Comment 4

[Patrick McManus [:mcmanus]]

  https://hg.mozilla.org/integration/mozilla-inbound/rev/c86cd310bee7

will want to backport this after it gets to m-c

Comment 5

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

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

Comment 6

[Patrick McManus [:mcmanus]]

Comment on attachment 8435954 [details] [diff] [review]
0001-bug-1021396-duplicate-spdy-syn_stream-can-hang-necko.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): original spdy support bug (ff13?)
User impact if declined: a server bug can lead to a complete networking hang in firefox.
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): live with the risk - requires a server bug to trigger which is not known to widely exist on the internet
String or IDL/UUID changes made by this patch: none

more info: If a server sends a double spdy syn_reply for the same stream it can result in the networking thread getting into an infinite loop. There is only one networking thread, so the result is that firefox networking stops and requires a session restart to fix.

This trigger is a server bug - so we don't see it from the big spdy properties on the Internet. I've seen the node.js based spdyproxy do this however (which is how I found this bug). I have only seen that in conjuction with spdy proxying (introduced in firefox-32 (now aurora)) but I cannot say with confidence whether or not the bug can only happen in that scenario - so its best imo to backport this to 31 as well.

http/2 is not impacted - just spdy/3 and spdy/3.1

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8435954 [details] [diff] [review]
0001-bug-1021396-duplicate-spdy-syn_stream-can-hang-necko.patch

Since we have this bug for a while, I won't take this change for beta.

Comment 8

[Patrick McManus [:mcmanus]]

 https://hg.mozilla.org/releases/mozilla-aurora/rev/4b243b08cdfb