Closed Bug 1485652 Opened 2 years ago Closed 4 months ago

Make nsNSSSocketInfo::IsAcceptableForHost work asynchronously on the socket process

Categories

(Core :: Networking: HTTP, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla77
Tracking Status
firefox63 --- wontfix
firefox77 --- fixed

People

(Reporter: mayhemer, Assigned: kershaw)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-triaged] )

Attachments

(2 files)

This is used for h2 conn joining an performs the cert chain verification, synchronously on the socket thread.

This method is going to be called on the socket process.  Certificate validation can only be done on the parent process, hence we need to IPC it, ideally asynchronously, in the worst case synchronously.

This will have two parts, if done async:
- PSM, should be fairly easy
- Necko, must wait for the result asynchronously

The Necko async part is going to be quite complicated.  Usage:

nsHttpConnectionMgr::TryDispatchTransaction,
nsHttpConnectionMgr::ProcessSpdyPendingQ,
nsHttpConnectionMgr::GetOrCreateConnectionEntry,
nsHttpConnectionMgr::nsConnectionEntry::AvailableForDispatchNow

all call it via:

nsHttpConnectionMgr::GetSpdyActiveConn -> nsHttpConnectionMgr::FindCoalescableConnection -> nsHttpConnectionMgr::FindCoalescableConnectionByHashKey -> nsHttpConnection -> ASpdySession -> nsISSLSocketControl.  


Patrick, Nick, do you happen to know about some way around it?  Note that right now we are already doing the verification on the socket process in a blocking way.  If we do a sync call to the parent and do it to a dedicated thread that can't be blocked on any sync calls to the socket process, we are probably safe.
Flags: needinfo?(mcmanus)
Flags: needinfo?(hurley)
Can you elaborate what you mean by "some way around it"? Do you mean some way of getting around the call into nsISSLSocketControl? Unfortunately, I don't believe so - all that info is required to know if we can merge the connections. We already coalesce more than other browsers do (though within the bounds of what's allowed by the spec), which leads to periodic confusion on the part of server implementors (and bug reports that I have to close). If we try to get clever, I'm afraid it will be far too easy to fall into a situation where we are improperly coalescing, and then we'll have real bugs to fix.

But, perhaps I'm misunderstanding your question.

(FTR, I think what you propose in your last sentence will work fine.)
Flags: needinfo?(hurley)
What I would prefer is to turn the synchronous bool getter "IsAcceptableForHost" to be async.  But returning the same result as we do now.  I.e. to change the API, not the decision.

This is probably going to be very hard as the stack is deep and the code complicated.

Hence, doing this in a sync way is probably the way to go.  If there are issues, we will deal with them later.

I'm moving this to necko since the hard work would happen there.
Component: Security: PSM → Networking: HTTP
Flags: needinfo?(mcmanus)
Priority: -- → P5
Whiteboard: [necko-triaged] maybe WONTFIX
(In reply to Honza Bambas (:mayhemer) from comment #2)
> Hence, doing this in a sync way is probably the way to go.  If there are

I mean through *sync IPC*
See Also: → 1537761
Assignee: nobody → kershaw
Priority: P5 → P2
Whiteboard: [necko-triaged] maybe WONTFIX → [necko-triaged]
Attachment #9135266 - Attachment description: Bug 1485652 - Implement remote IsAcceptableForHost → Bug 1485652 - Reimplement IsAcceptableForHost
Pushed by kjang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0774925b94a9
Reimplement IsAcceptableForHost r=keeler
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
You need to log in before you can comment on or make changes to this bug.