1398646 - DNS service synchronous resolution methods should not work on the main thread

Status: RESOLVED FIXED

(Core :: Networking: DNS, defect)

Description

[Kris Maglione [:kmag]]

I just reviewed a patch that tried to use synchronous DNS resolution on the main thread. The fact that is even possible is a major performance footgun that should be fixed.

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1398646: Forbid sync DNS resolution on the main thread.

Review commit: https://reviewboard.mozilla.org/r/178148/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/178148/

Comment 2

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

Thanks for looking into this. I think this might even fix a recent bug - that connecting to slow DNS servers blocks startup.

Comment 3

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

Comment on attachment 8906430 [details]
Bug 1398646: Forbid sync DNS resolution on the main thread.

https://reviewboard.mozilla.org/r/178148/#review183222

Comment 4

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/963bf9d8573273894016e7082cb5f3fb85d80661
Bug 1398646: Forbid sync DNS resolution on the main thread. r=valentin

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/963bf9d85732

Comment 6

[will69]

This has caused a regression. See bug #1400288.

Comment 7

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5473323817b8b412f492307ce3b7a2bd72f59b21
Backed out changeset 963bf9d85732 (bug 1398646) for causing bug 1400288. r=backout

Comment 8

[Kris Maglione [:kmag]]

Comment on attachment 8906430 [details]
Bug 1398646: Forbid sync DNS resolution on the main thread.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/178148/diff/1-2/

Comment 9

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

Comment on attachment 8906430 [details]
Bug 1398646: Forbid sync DNS resolution on the main thread.

https://reviewboard.mozilla.org/r/178148/#review186238

Let's add a comment to nsIDNSService mentioning that resolveNative returns an error if called on the main thread.
r=me

Comment 10

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1303bbfb017d0a7446b959c9fcbe5bb65f296950
Bug 1398646: Forbid sync DNS resolution on the main thread. r=valentin

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/1303bbfb017d