Last Comment Bug 656989 - nsHttpConnection doesn't change mCallbacks to actual during Activate
: nsHttpConnection doesn't change mCallbacks to actual during Activate
Status: RESOLVED FIXED
[http-conn]
:
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Patrick McManus [:mcmanus]
:
: Patrick McManus [:mcmanus]
Mentors:
Depends on:
Blocks: 623134 623948 651635
  Show dependency treegraph
 
Reported: 2011-05-13 12:40 PDT by Honza Bambas (:mayhemer)
Modified: 2012-06-21 06:42 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
update security callbacks v1 (1.51 KB, patch)
2011-05-13 13:11 PDT, Patrick McManus [:mcmanus]
mcmanus: review+
Details | Diff | Splinter Review

Description Honza Bambas (:mayhemer) 2011-05-13 12:40:11 PDT
Discovered by Patrick McManus during work on bug 635545 comment 30.

This is a regression from bug 623948.

The issue is that mCallbacks is assigned to nsHttpConnection object at the time it is first established.  There are two ways we handle such connection: we activate it with a transaction from which the callbacks were originally extracted (most common, and all good) or, we put it on the idle list leaving it with the original callbacks.

It the letter case, or in case of reusing the connection in the former case, we do not properly change the callbacks to callbacks of the new transaction that the connection is just being (re)activated with.
Comment 1 Patrick McManus [:mcmanus] 2011-05-13 13:05:20 PDT
While running the tests involved in 654201 I sometimes would get a failure in browser/components/privatebrowsing/test/browser/browser_privatebrowsing_certexceptionsui.js from browser-chrome.

setting the syn retry timeout to 1 and running just that single mochitest made it 50% likely to reproduce. The mochitest would timeout as a failure mode because there was no nsibadsslcertlistener available when it was needed.

The root cause of that was out of date security callbacks that were not reset when an extra idle persistent connection was used for a different transaction than the one that initiated it.

we have an r+'d patch attached to that other bug that I am going to transfer over here - so it can be attached to a bug report of its own as it is really a separate issue.
Comment 2 Patrick McManus [:mcmanus] 2011-05-13 13:11:54 PDT
Created attachment 532328 [details] [diff] [review]
update security callbacks v1

r=honzab
Comment 3 Daniel Holbert [:dholbert] 2011-05-13 13:37:03 PDT
Pushed to cedar (after removing the 4 trailing spaces on the final line added in this bug's patch):
  http://hg.mozilla.org/projects/cedar/rev/6d0e02e42e34

After I pushed, I also noticed that the commit message had the wrong bug number -- it said "bug 654201".  I'll post a comment on that bug pointing over here.
Comment 4 Jason Duell [:jduell] (needinfo me) 2011-05-13 19:12:09 PDT
http://hg.mozilla.org/mozilla-central/rev/6d0e02e42e34

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