Skip to content

Back off connectivity probes under descriptor exhaustion #996

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kasnder wants to merge 1 commit into from
Closed

Back off connectivity probes under descriptor exhaustion #996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions app/src/main/java/eu/faircode/netguard/ProbeResourceGuard.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package eu.faircode.netguard;

import android.system.ErrnoException;
import android.system.Os;
import android.system.OsConstants;

import java.io.File;

/** Limits optional connectivity probes when the process cannot allocate descriptors. */
final class ProbeResourceGuard {
// Process-wide: losing a Network or recreating the service must not bypass the cooldown.
static final ProbeResourceGuard INSTANCE = new ProbeResourceGuard();
static final long COOLDOWN_MS = 10 * 60 * 1000L;
static final int MAX_SAMPLED_DESCRIPTORS = 256;
private long retryAfterMs;

synchronized boolean isBlocked(long nowMs, boolean interactive) {
return retryAfterMs != 0 && (nowMs < retryAfterMs || !interactive);
}

/** Returns true only for the first failure in a cooldown, to bound diagnostics too. */
synchronized boolean onExhaustion(long nowMs) {
if (nowMs < retryAfterMs)
return false;
retryAfterMs = nowMs + COOLDOWN_MS;
return true;
}

synchronized void onSuccess(long nowMs) {
// A successful probe already in flight must not cancel a newly started cooldown.
if (nowMs >= retryAfterMs)
retryAfterMs = 0;
}

static boolean isDescriptorExhaustion(Throwable failure) {
// Android wraps errno in IOException and sometimes UncheckedIOException.
// Bound traversal defensively; never classify by a possibly misleading message.
for (int depth = 0; failure != null && depth < 16; depth++, failure = failure.getCause()) {
if (failure instanceof ErrnoException) {
int errno = ((ErrnoException) failure).errno;
if (errno == OsConstants.EMFILE || errno == OsConstants.ENFILE)
return true;
}
}
return false;
}

static String descriptorSummary() {
try {
return descriptorSummary(new File("/proc/self/fd").list(), Os::readlink);
} catch (SecurityException ex) {
return "fd census unavailable";
}
}

interface LinkReader {
String read(String path) throws ErrnoException;
}

static String descriptorSummary(String[] descriptors, LinkReader reader) {
// Opening /proc/self/fd can itself fail under EMFILE. Do not reserve an FD or retry.
if (descriptors == null)
return "fd census unavailable";
int sampled = Math.min(descriptors.length, MAX_SAMPLED_DESCRIPTORS);
int sockets = 0, pipes = 0, anonymous = 0, other = 0, unreadable = 0;
for (int i = 0; i < sampled; i++) {
try {
String target = reader.read("/proc/self/fd/" + descriptors[i]);
if (target.startsWith("socket:"))
sockets++;
else if (target.startsWith("pipe:"))
pipes++;
else if (target.startsWith("anon_inode:"))
anonymous++;
else
other++;
} catch (ErrnoException | SecurityException ex) {
// Descriptors may close during the census. Never log targets or exceptions.
unreadable++;
}
}
return "fd total=" + descriptors.length + " sampled=" + sampled
+ " sockets=" + sockets + " pipes=" + pipes + " anonymous=" + anonymous
+ " other=" + other + " unreadable=" + unreadable;
}
}
35 changes: 27 additions & 8 deletions app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.io.InputStreamReader;
import java.net.Inet4Address;
import java.net.Inet6Address;
Expand Down Expand Up @@ -3431,6 +3432,10 @@ public void onUnavailable() {
}

private void checkConnectivity(Network network, NetworkInfo ni, NetworkCapabilities capabilities) {
if (ProbeResourceGuard.INSTANCE.isBlocked(SystemClock.elapsedRealtime(),
Util.isInteractive(ServiceSinkhole.this)))
return;

if (isActiveNetwork(network) &&
ni != null && capabilities != null &&
ni.getDetailedState() != NetworkInfo.DetailedState.SUSPENDED &&
Expand Down Expand Up @@ -3474,6 +3479,10 @@ private void checkConnectivity(Network network, NetworkInfo ni, NetworkCapabilit
@Override
public void run() {
try {
// A queued probe must also honour exhaustion detected by another probe.
if (ProbeResourceGuard.INSTANCE.isBlocked(SystemClock.elapsedRealtime(),
Util.isInteractive(ServiceSinkhole.this)))
return;
SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(ServiceSinkhole.this);
String host = prefs.getString("validate", "www.f-droid.org");
Log.i(TAG, "Validating " + network + " " + ni + " host=" + host);
Expand All @@ -3487,9 +3496,11 @@ public void run() {
}

Socket socket = null;
boolean captureResourceDiagnostics = false;
try {
socket = network.getSocketFactory().createSocket();
socket.connect(new InetSocketAddress(host, 443), 10000);
ProbeResourceGuard.INSTANCE.onSuccess(SystemClock.elapsedRealtime());
Log.i(TAG, "Validated " + network + " " + ni + " host=" + host);
synchronized (mapValidated) {
mapValidated.put(network, new Date().getTime());
Expand All @@ -3498,21 +3509,29 @@ public void run() {
ConnectivityManager cm = (ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE);
cm.reportNetworkConnectivity(network, true);
Log.i(TAG, "Reported " + network + " " + ni);
} catch (IOException ex) {
Log.e(TAG, ex.toString());
Log.i(TAG, "No connectivity " + network + " " + ni);
long[] previousFailure = mapValidateFailure.get(network);
long previousBackoffMs = previousFailure == null ? 0L : previousFailure[1];
mapValidateFailure.put(network, new long[]{
SystemClock.elapsedRealtime(),
nextValidationBackoffMs(previousBackoffMs)});
} catch (IOException | UncheckedIOException ex) {
if (ProbeResourceGuard.isDescriptorExhaustion(ex)) {
captureResourceDiagnostics = ProbeResourceGuard.INSTANCE.onExhaustion(
SystemClock.elapsedRealtime());
} else {
Log.e(TAG, ex.toString());
Log.i(TAG, "No connectivity " + network + " " + ni);
long[] previousFailure = mapValidateFailure.get(network);
long previousBackoffMs = previousFailure == null ? 0L : previousFailure[1];
mapValidateFailure.put(network, new long[]{
SystemClock.elapsedRealtime(),
nextValidationBackoffMs(previousBackoffMs)});
}
} finally {
if (socket != null)
try {
socket.close();
} catch (IOException ex) {
Log.e(TAG, ex.toString() + "\n" + Log.getStackTraceString(ex));
}
if (captureResourceDiagnostics)
Log.w(TAG, "Probe descriptor exhaustion (#932); probes paused for 10 minutes; "
+ ProbeResourceGuard.descriptorSummary());
}
} finally {
validating.remove(network);
Expand Down
91 changes: 91 additions & 0 deletions app/src/test/java/eu/faircode/netguard/ProbeResourceGuardTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package eu.faircode.netguard;

import static org.junit.Assert.*;

import android.system.ErrnoException;
import android.system.OsConstants;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.concurrent.atomic.AtomicInteger;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner;

@RunWith(RobolectricTestRunner.class)
public class ProbeResourceGuardTest {
@Test
public void recognisesCheckedAndUncheckedDescriptorFailures() {
for (int errno : new int[]{OsConstants.EMFILE, OsConstants.ENFILE}) {
IOException checked = new IOException(new ErrnoException("fcntl", errno));
assertTrue(ProbeResourceGuard.isDescriptorExhaustion(checked));
assertTrue(ProbeResourceGuard.isDescriptorExhaustion(new UncheckedIOException(checked)));
}
assertFalse(ProbeResourceGuard.isDescriptorExhaustion(
new IOException(new ErrnoException("connect", OsConstants.ETIMEDOUT))));
assertFalse(ProbeResourceGuard.isDescriptorExhaustion(new IOException("EMFILE")));
}

@Test
public void cooldownSuppressesAllProbesAndDiagnosticsUntilBoundary() {
ProbeResourceGuard guard = new ProbeResourceGuard();
assertFalse(guard.isBlocked(100, false));
assertTrue(guard.onExhaustion(100));
assertTrue(guard.isBlocked(101, true));
assertFalse(guard.onExhaustion(101));
assertTrue(guard.isBlocked(100 + ProbeResourceGuard.COOLDOWN_MS - 1, true));
assertFalse(guard.isBlocked(100 + ProbeResourceGuard.COOLDOWN_MS, true));
// Duplicate failures neither extend the cooldown nor allow duplicate diagnostics.
assertTrue(guard.onExhaustion(100 + ProbeResourceGuard.COOLDOWN_MS));
}

@Test
public void expiredCooldownStillDefersScreenOffRetriesUntilRecovery() {
ProbeResourceGuard guard = new ProbeResourceGuard();
guard.onExhaustion(100);
long later = 100 + ProbeResourceGuard.COOLDOWN_MS;
assertTrue(guard.isBlocked(later, false));
assertFalse(guard.isBlocked(later, true));
guard.onSuccess(later);
assertFalse(guard.isBlocked(later, false));
}

@Test
public void inFlightSuccessCannotClearNewCooldown() {
ProbeResourceGuard guard = new ProbeResourceGuard();
guard.onExhaustion(100);
guard.onSuccess(101);
assertTrue(guard.isBlocked(102, true));
}

@Test
public void censusContainsOnlyAggregateCounts() {
String[] targets = {"socket:[123]", "pipe:[456]", "anon_inode:[eventpoll]",
"/private/sensitive-file", "/another/private-file"};
AtomicInteger index = new AtomicInteger();
String result = ProbeResourceGuard.descriptorSummary(new String[]{"0", "1", "2", "3", "4", "5"},
path -> {
int i = index.getAndIncrement();
if (i == targets.length)
throw new ErrnoException("private diagnostic text", OsConstants.ENOENT);
return targets[i];
});
assertEquals("fd total=6 sampled=6 sockets=1 pipes=1 anonymous=1 other=2 unreadable=1", result);
}

@Test
public void censusBoundsReadlinkWorkAndHandlesDirectoryFailure() {
AtomicInteger reads = new AtomicInteger();
String result = ProbeResourceGuard.descriptorSummary(new String[1000], path -> {
reads.incrementAndGet();
return "socket:[123]";
});
assertEquals(256, reads.get());
assertEquals("fd total=1000 sampled=256 sockets=256 pipes=0 anonymous=0 other=0 unreadable=0", result);
assertEquals("fd census unavailable", ProbeResourceGuard.descriptorSummary(null, path -> {
fail("Must not read links when the directory is unavailable");
return "";
}));
}
}
Loading