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
Last Comment Bug 758972 - Is Spdy VerifyStream() needed?
: Is Spdy VerifyStream() needed?
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: unspecified
: x86_64 Windows 7
: -- enhancement (vote)
: mozilla17
Assigned To: Patrick McManus [:mcmanus]
: Patrick McManus [:mcmanus]
Depends on: 759438
  Show dependency treegraph
Reported: 2012-05-27 10:22 PDT by Patrick McManus [:mcmanus]
Modified: 2012-07-20 21:06 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch 0 (1.77 KB, patch)
2012-07-19 09:11 PDT, Patrick McManus [:mcmanus]
honzab.moz: review+
Details | Diff | Splinter Review

Description User image Patrick McManus [:mcmanus] 2012-05-27 10:22:24 PDT
bug 753663 tracked a crash where a invalid spdy stream was used.. generally under the signature mozilla::net::SpdyStream::UpdateTransportReadEvents 

the source of the crash has never been fully understood.

bug 756551 fixed that crash through a series of integrity checks and theoretically un-needed intializations.

when we have a full nightly window we should change VerifyStream() to be DEBUG only and see if the crashes have been genuinely fixed or not via nightly crash stats. (we can expect about 1 a day on nightly.. 4+ or so with a current sized aurora audience).
Comment 1 User image Patrick McManus [:mcmanus] 2012-07-19 07:37:50 PDT
I believe 759438 was the underlying cause of the corruption.

Given that we have a fresh nightly cycle I think we should turn VerifyStream() into a DEBUG-build only feature and look for regressions in crash stats.
Comment 2 User image Patrick McManus [:mcmanus] 2012-07-19 09:11:23 PDT
Created attachment 643879 [details] [diff] [review]
patch 0
Comment 3 User image Honza Bambas (:mayhemer) 2012-07-19 15:15:41 PDT
Comment on attachment 643879 [details] [diff] [review]
patch 0

Review of attachment 643879 [details] [diff] [review]:

::: netwerk/protocol/http/SpdySession3.cpp
@@ +704,5 @@
> +#ifndef DEBUG
> +  // Only do the real verification in debug builds
> +  return true;
> +#endif
> +  fprintf(stderr,"VERIFY\n");

fprintf leftover?
Comment 4 User image Patrick McManus [:mcmanus] 2012-07-20 05:51:32 PDT
(In reply to Honza Bambas (:mayhemer) from comment #3)

> fprintf leftover?

doh! thanks.
Comment 5 User image Patrick McManus [:mcmanus] 2012-07-20 06:45:24 PDT
Comment 6 User image Ryan VanderMeulen [:RyanVM] 2012-07-20 21:06:36 PDT

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