Open Bug 1798505 Opened 2 years ago Updated 2 years ago

nsINetworkConnectivityService should not have a race condition when determining network connectivity

Categories

(Core :: Networking, defect, P3)

defect

Tracking

()

People

(Reporter: tgiles, Unassigned)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [necko-triaged])

Currently, as :kershaw has previously pointed out in Bug 1793498, using the nsINetworkConnectivityService might not have accurate and reliable information when trying to reason about the internet connectivity of a particular Firefox instance. Now I'm not sure why this particular check, say using nsINetworkConnectivityService.IPv4, is a race condition and I'm hoping that someone can either point me in the right direction to understanding this issue or explain the issue so I can understand. I wouldn't be surprised if this is a very large problem and I've just looked at it from the wrong spot :) Please let me know what I'm missing.

Would it be possible to do something like :gijs has described on Bug 1789872 Comment 14?

If there's no network then the http response can't come back to us. So it has to be the case that there is a network link change after the successful http response arrives, and that should invalidate the status on the network connectivity service, I think.
(even if this is racy, then we should detect a network change that happens while we're waiting for the http response and just discard that response and try again)

Having an accurate and reliable way for the front-end to determine if we're connected to the internet or not would be extremely beneficial.

The way nsINetworkConnectivityService is that after a network change event we will perform the connectivity checks and until they are done the states will be ConnectivityState::UNKNOWN, even though connectivity state hasn't changed.

I assume you could use it by listening for the network:connectivity-service:ip-checks-complete observer notification and checking the state then? Of course, it would be good not to block anything waiting for it.

+1 to valentin's comment.

We can (as Kershaw indicated) accurately tell if network links are up or down.

Accurately knowing if we're "connected to the internet" is much tougher and inherently fuzzy. For example, if you're on wifi and are on the edge of reception, very few packets may get through (and so it "works" but is very, very slow - perhaps 10's of seconds to get a response), or they may get through for 10 seconds and then not get through for 60 seconds. Are you connected to the internet? When should that change? How do you monitor this without constantly sending packets (which is problematic for many reasons).

When a link status changes, we attempt to revalidate the connectivity; link status changes are relatively rare. Realize that any status change may be delayed, perhaps by a fair bit.

Also: what if the "internet" is up, but the firefox detect portal is down/unreachable? What if it's simply blocked, or overloaded, or DDOSed? Will we report no connectivity? Probably. (I haven't checked the code.) We should ensure that we don't block important functionality based on an unreliable indicator (though we could change some behaviors/UI).

(In reply to Valentin Gosu [:valentin] (he/him) from comment #1)

I assume you could use it by listening for the network:connectivity-service:ip-checks-complete observer notification and checking the state then? Of course, it would be good not to block anything waiting for it.

This might work for the Firefox View case. We wouldn't be blocking anything waiting for that check to complete. We would just wait for the check to finish, inspect the state, then if we're online again, update the Firefox View error message. I can try this out and see if this keeps us moving on the dependent bug (1789872).

(In reply to Randell Jesup [:jesup] (needinfo me) from comment #2)

+1 to valentin's comment.
We can (as Kershaw indicated) accurately tell if network links are up or down.
Accurately knowing if we're "connected to the internet" is much tougher and inherently fuzzy.

Snipping a bit just so the quoted message isn't too long. Thank you both for the explanations and clarification! I assumed this was a large/tricky issue and that assumption appears to be correct. It seems like I could hack something together to solve the dependent bug (as previously stated) but it seems like there's no "perfect" solution to the issue.

Not sure what to do with this bug, it seems like there isn't a way for nsINetworkConnectivityService to not have a race condition when determining network connectivity...and it seems like listening to network:connectivity-service:ip-checks-complete is a way to move forward but maybe not a pattern we want to start introducing into the front-end code.

Blocks: 1567627
Severity: -- → S4
Priority: -- → P3
Whiteboard: [necko-triaged]
You need to log in before you can comment on or make changes to this bug.