nsHttpConnection doesn't change mCallbacks to actual during Activate

RESOLVED FIXED

Status

()

Core
Networking: HTTP
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: mayhemer, Assigned: mcmanus)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [http-conn])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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.
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.
Created attachment 532328 [details] [diff] [review]
update security callbacks v1

r=honzab
Assignee: nobody → mcmanus
Attachment #532328 - Flags: review+
Keywords: checkin-needed
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.
Keywords: checkin-needed
Whiteboard: fixed-in-cedar
http://hg.mozilla.org/mozilla-central/rev/6d0e02e42e34
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Blocks: 651635
Whiteboard: fixed-in-cedar
Blocks: 623134
(Reporter)

Updated

5 years ago
Whiteboard: [http-conn]
You need to log in before you can comment on or make changes to this bug.