Note: There are a few cases of duplicates in user autocompletion which are being worked on.

make nsCertOverrideService's locking more efficient

RESOLVED FIXED in Firefox 55

Status

()

Core
Security: PSM
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

nsCertOverrideService uses a ReentrantMonitor to protect its inner
state.  However, there's no way for nsCertOverrideService's methods to
be re-entered when calling outside code.  The use of ReentrantMonitor
appears to be compensating for an unclear division of locking
responsibilities, by enabling every method to simply lock the
ReentrantMonitor upon entrance without care for who might have locked it
beforehand.

Using Mutex is cheaper than ReentrantMonitor, and also forces us to
make explicit who's required to do locking, and who needs to do work
with the lock held.
Created attachment 8859581 [details] [diff] [review]
make nsCertOverrideService's locking more efficient

Try at least seems to think this patch is OK:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f180f607141d4e6013caa861be83dca3e1b7ea53
Attachment #8859581 - Flags: review?(dkeeler)
Comment on attachment 8859581 [details] [diff] [review]
make nsCertOverrideService's locking more efficient

Review of attachment 8859581 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM - thanks for writing this.

::: security/manager/ssl/nsCertOverrideService.h
@@ +159,5 @@
>  
>  protected:
>      ~nsCertOverrideService();
>  
> +    mozilla::Mutex mutex;

nit: maybe let's call this mMutex to be consistent with the other member variables
Attachment #8859581 - Flags: review?(dkeeler) → review+

Comment 3

3 months ago
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8228b51bee64
make nsCertOverrideService's locking more efficient; r=keeler

Comment 4

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8228b51bee64
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.