If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

nsSocketTransport::KeepWhenOffline and nsSocketTransport::InitiateSocket don't agree

RESOLVED INVALID

Status

()

Core
Networking
RESOLVED INVALID
4 years ago
4 years ago

People

(Reporter: jimb, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

4 years ago
nsSocketTransport is of two minds in different spots about whether local sockets should be kept open when offline.

When IOService is set to offline mode, among other things, it tells nsSocketTransportService to take itself offline. When the STS goes offline, it detaches all its sockets --- except those which, according to their handlers, are 1) local, and 2) should be kept when offline. (This is all nsSocketTransportService::DetachSocketWithGuard, in netwerk/base/src/nsSocketTransportService2.cpp.)

The nsASocketHandler class, which nsSocketTransport inherits from, declares the IsLocal and KeepWhenOffline member functions. The default definition of KeepWhenOffline says "no" --- local sockets should be detached when we go offline, unless the derived class overrides that. nsSocketTransport does not.

However, while nsSocketTransportService::InitiateSocket does check gIOService->IsOffline for the purpose of returning an NS_ERROR_OFFLINE code, it explicitly *permits* sockets with local addresses to proceed.

Perhaps I'm missing something really subtle here, but that combination of behaviors doesn't make any sense. KeepWhenOffline is only overridden for server sockets; there isn't any way to create nsSocketTransports whose KeepWhenOffline returns "yes, please." And even if InitiateSocket were to allow a local socket's creation to go forward when the STS was offline, I would expect that putting the STS online and then taking it offline again would detach the socket anyway.

Since the check in InitiateSocket was explicitly added in the fix for bug 87717 (not a typo), I assume that code actually relies on that behavior: nsSocketTransport instances for local addresses should always be kept around in offline mode. That suggests that the fix is for nsSocketTransport to override KeepWhenOffline to return true, to protect the sockets when offline.
(Reporter)

Comment 1

4 years ago
The test to write, here, would be one that creates a local socket while offline, and then switches IOService back online and then offline, and verifies that the local socket is unaffected.
(Reporter)

Comment 2

4 years ago
Rats. I wasn't missing something subtle, I was missing the '!' in nsSocketTransportService::DetachSocketWithGuard.

Sockets are retained if their are local *or* if they should be kept when offline. So the test in InitiateSocket is correct in assuming that, if the socket is local, it should be created even if we are offline; and KeepWhenOffline definitely should *not* be overridden, lest all transports be retained.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.