Last Comment Bug 758972 - Is Spdy VerifyStream() needed?
: Is Spdy VerifyStream() needed?
Status: RESOLVED FIXED
[spdy]
:
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: unspecified
: x86_64 Windows 7
: -- enhancement (vote)
: mozilla17
Assigned To: Patrick McManus [:mcmanus]
:
Mentors:
Depends on: 759438
Blocks:
  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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


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

Description 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 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 Patrick McManus [:mcmanus] 2012-07-19 09:11:23 PDT
Created attachment 643879 [details] [diff] [review]
patch 0
Comment 3 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 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 Patrick McManus [:mcmanus] 2012-07-20 06:45:24 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/43f259b15ccc
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-07-20 21:06:36 PDT
https://hg.mozilla.org/mozilla-central/rev/43f259b15ccc

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