As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 798423 - spdy enable reading of ping reply
: spdy enable reading of ping reply
Status: RESOLVED FIXED
[spdy][qa?]
:
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: 16 Branch
: x86_64 Linux
: -- normal (vote)
: mozilla18
Assigned To: Patrick McManus [:mcmanus]
:
: Patrick McManus [:mcmanus]
Mentors:
Depends on: 823030 819044
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-05 09:41 PDT by Patrick McManus [:mcmanus]
Modified: 2012-12-21 14:52 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
patch 0 (1.48 KB, patch)
2012-10-05 10:01 PDT, Patrick McManus [:mcmanus]
honzab.moz: review+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description User image Patrick McManus [:mcmanus] 2012-10-05 09:41:39 PDT
I noticed when reading logs for bug 790184 that some idle spdy sessions were failing their keep alive checks when networking appeared to be working fine.

it turns out when we generate a ping sometimes mSocketIn is not set to read data and so the ping reply is queued without being read and eventually the session times out.

This results in tearing down idle sessions more aggressively than we intend to.
Comment 1 User image Patrick McManus [:mcmanus] 2012-10-05 10:01:41 PDT
Created attachment 668489 [details] [diff] [review]
patch 0
Comment 2 User image Honza Bambas (:mayhemer) 2012-10-05 11:32:50 PDT
Comment on attachment 668489 [details] [diff] [review]
patch 0

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

Ooops!  This was really missing!
Comment 3 User image Patrick McManus [:mcmanus] 2012-10-05 13:20:24 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/39d7c39c6ac2
Comment 4 User image Ed Morley [:emorley] 2012-10-06 12:50:39 PDT
https://hg.mozilla.org/mozilla-central/rev/39d7c39c6ac2
Comment 5 User image Patrick McManus [:mcmanus] 2012-10-08 06:34:17 PDT
Comment on attachment 668489 [details] [diff] [review]
patch 0

790184 was r? for FF17, and this patch should go with it to ensure full efficacy. This patch is very low risk.
Comment 6 User image Patrick McManus [:mcmanus] 2012-10-08 20:01:37 PDT
  https://hg.mozilla.org/releases/mozilla-beta/rev/8bc762ccc4ea
Comment 7 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-10-17 15:59:41 PDT
Is there something QA can do here to verify this is fixed?
Comment 8 User image Ed Morley [:emorley] 2012-12-21 14:52:22 PST
(These dependants were mis-added to bug 798243 due to the wrong bug number being in the commit message)

Note You need to log in before you can comment on or make changes to this bug.