1677501 - PK11_ListCerts called by DoHHeuristics.jsm during late startup does a lot of main thread I/O to cert9.db

Status: RESOLVED FIXED

(Core :: Networking: DNS, defect, P2)

Description

[Florian Quèze [:florian]]

When looking at BHR data from our nightly users, PK11_ListCerts represents more than 1% of the total hang time. (http://queze.net/bhr/test/#row=&filter=PK11_ListCerts)

The stacks show that some JS code calls nsNSSCertificateDB::GetCerts. Most stacks aren't very helpful after that, mostly showing that we are resuming from within an async function.

A few of the stacks reported through BHR show /modules/DoHHeuristics.jsm:221 which points to https://searchfox.org/mozilla-central/rev/ff82c973f8ccb0475ec32439e9ec07014b3a681f/browser/components/doh/DoHHeuristics.jsm#221,230

After setting the doh-rollout.enabled pref to true in about:config, I could reproduce on my slow reference laptop. Here's what this looks like in a startup profile with that preference enabled: https://share.firefox.dev/3ntRYOX

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Excellent find, Florian. Thanks for this report.

Dana, do you think we can replace this code with something that doesn't impact performance?
Maybe we can just check these prefs

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

security.enterprise_roots.enabled is already checked, and security.enterprise_roots.auto-enabled is only relevant insofar as it can set security.enterprise_roots.enabled to true.
Basically, if you want to check if there are any third-party trust anchors in the on-disk certificate database, you have to read the on-disk certificate database. One option could be to not do that. The other option would be to make getCerts() asynchronous, so at least the main thread isn't blocked. Note that this change would also improve main-thread-blocking in other cases, such as the certificate manager.

Comment 3

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1677501 - Add nsIX509CertDB.asyncGetCerts r=keeler

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1677501 - Use nsIX509CertDB.asyncGetCerts in DoHHeuristics.jsm to avoid main thread IO r=Gijs

Depends on D97425

Comment 5

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1677501 - Add nsIX509CertDB.asyncHasThirdPartyRoots and use it in DoHHeuristics.jsm r=keeler!,Gijs!

This allows us to avoid calling any NSSCertificateDB methods on the main
thread or allocating memory for xpconnect wrappers of cert objects.

Comment 6

[Phabricator Automation]

Comment on attachment 9188549 [details]
Bug 1677501 - Add nsIX509CertDB.asyncGetCerts r=keeler

Revision D97425 was moved to bug 1679954. Setting attachment 9188549 [details] to obsolete.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7351aa88de95
Add nsIX509CertDB.asyncHasThirdPartyRoots and use it in DoHHeuristics.jsm r=keeler,Gijs,nhnt11

Comment 8

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/7351aa88de95