1800603 - Setting network.captive-portal-service.enabled to false and immediately after that to true leads to crash

Status: VERIFIED FIXED

(Core :: Networking, defect, P2)

Description

[Alexander Gavrilov [:dartraiden]]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:107.0) Gecko/20100101 Firefox/107.0

Steps to reproduce:

Create a user.js with just 2 strings:

user_pref("network.captive-portal-service.enabled", false);
user_pref("network.captive-portal-service.enabled", true);

Start Firefox after that

Actual results:

Crash. Tested on Firefox 107 under Windows/Linux and Firefox Nightly under Linux.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Networking' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Mayank Bansal]

Can repro.
https://crash-stats.mozilla.org/report/index/ebcc6d66-94e9-4fb2-b793-83b900221115

Comment 3

[Mayank Bansal]

Regression: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2c912e46295e4f8a3fa5824a7f378e06760ec7dd&tochange=d2d4fba86fce4ca4391e1886dd195b286d794ac0

Comment 4

[Kris Maglione [:kmag]]

This isn't that surprising. This causes the preference service to notify the captive portal service of a preference change, which causes the captive portal service to try to load a JSM. But this is too early to run JS.

We should maybe try not to notify of pref changes this early.

Comment 5

[Valentin Gosu [:valentin] {{ IETF until 25 July - PTO until 3 August }}]

This looks like a regression from bug 1579367.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1579367

Comment 7

[Jan de Mooij [:jandem]]

(In reply to Kris Maglione [:kmag] from comment #4)

We should maybe try not to notify of pref changes this early.

You mean checking for this in NotifyCallbacks? I'm not familiar with this at all so I don't know what the implications are of changing that...

Could we initialize the nsIOService after loading these prefs?

Comment 8

[Valentin Gosu [:valentin] {{ IETF until 25 July - PTO until 3 August }}]

I don't know if we can change when nsIOService is created.
We can fix just this individual case maybe, if we have a way of knowing when it's OK to run JS.
But there might be other prefs for which we trigger JS.

Comment 9

[Kris Maglione [:kmag]]

I think we probably have to do it case-by-case. I don't think we have a way to check whether we're ready to run JS, but we can add one. Or the captive portal code could just add a flag when it gets the "app-startup" notification, and only handle pref changes afterward.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1579367

Comment 11

[Chris Peterson [:cpeterson]]

We have some of these CaptivePortalService crashes from Fenix 107.1.0 Release, which doesn't have an about:config page available to users. Is there another way this crash can be triggered besides toggling the network.captive-portal-service.enabled pref?

https://crash-stats.mozilla.org/report/index/63b1ea58-7a66-4836-ba42-ca1320221122

Comment 12

[Greg Hess]

Necko will take this one.

Comment 13

[Valentin Gosu [:valentin] {{ IETF until 25 July - PTO until 3 August }}]

Attachment: Bug 1800603 - Do not instantiate captive portal detector if it's too early to run JS r=#necko

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/40079b294d34
Do not instantiate captive portal detector if it's too early to run JS r=necko-reviewers,kershaw

Comment 15

[Serban Stanca [:SerbanS]]

Backed out for causing Xpcshell failures in test_ext_captivePortal.js.

• Backout link

• Push with failures

• Failure Log

• Failure line: TEST-UNEXPECTED-TIMEOUT | xpcshell-remote.ini:toolkit/components/extensions/test/xpcshell/test_ext_captivePortal.js | Test timed out

• It also failed on this.

• Failure Log

• Failure line: TEST-UNEXPECTED-TIMEOUT | netwerk/test/unit/test_trr.js | Test timed out

Comment 16

[Jan de Mooij [:jandem]]

Thanks for fixing this one, Valentin!

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ea17a6788b24
Do not instantiate captive portal detector if it's too early to run JS r=necko-reviewers,kershaw,jandem

Comment 18

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/ea17a6788b24

Comment 19

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Reproduced the issue with Firefox 109.0a1 (2022-11-15) on Windows 10x64 using the STR from comment 0. Firefox crashes when using the user.js.
The issue is verified fixed with Firefox 111.0b8 on Windows 10x64, macOS 12, and Ubuntu 20.04. Firefox no longer crashes after using steps from comment 0.