Trip the DoH circuit breaker per attempt and drop the dead delayed-screen-off alarm

app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java

@@ -316,7 +316,6 @@ private static ExecutorService createEpdgResolver() {
     }
 
     private static final String ACTION_HOUSE_HOLDING = "eu.faircode.netguard.HOUSE_HOLDING";
-    private static final String ACTION_SCREEN_OFF_DELAYED = "eu.faircode.netguard.SCREEN_OFF_DELAYED";
     private static final String ACTION_WATCHDOG = "eu.faircode.netguard.WATCHDOG";
 
     private native long jni_init(int sdk);
@@ -493,7 +492,6 @@ else if (cmd == Command.reload && temporarilyStopped) {
                     IntentFilter ifInteractive = new IntentFilter();
                     ifInteractive.addAction(Intent.ACTION_SCREEN_ON);
                     ifInteractive.addAction(Intent.ACTION_SCREEN_OFF);
-                    ifInteractive.addAction(ACTION_SCREEN_OFF_DELAYED);
                     ContextCompat.registerReceiver(ServiceSinkhole.this, interactiveStateReceiver, ifInteractive,
                             ContextCompat.RECEIVER_NOT_EXPORTED);
                     registeredInteractiveState = true;
@@ -2854,13 +2852,6 @@ public void onReceive(final Context context, final Intent intent) {
             executor.submit(new Runnable() {
                 @Override
                 public void run() {
-                    AlarmManager am = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE);
-                    Intent i = new Intent(ACTION_SCREEN_OFF_DELAYED);
-                    i.setPackage(context.getPackageName());
-                    PendingIntent pi = PendingIntentCompat.getBroadcast(context, 0, i,
-                            PendingIntent.FLAG_UPDATE_CURRENT);
-                    am.cancel(pi);
-
                     try {
                         last_interactive = Intent.ACTION_SCREEN_ON.equals(intent.getAction());
                         InteractiveStatePolicy.onScreenStateChanged(
@@ -2887,11 +2878,6 @@ public void onStatsInteractiveStateChanged(boolean interactive) {
                                 .onScreenStateChanged(last_interactive);
                     } catch (Throwable ex) {
                         Log.e(TAG, ex.toString() + "\n" + Log.getStackTraceString(ex));
-
-                        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M)
-                            am.set(AlarmManager.RTC_WAKEUP, new Date().getTime() + 15 * 1000L, pi);
-                        else
-                            am.setAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, new Date().getTime() + 15 * 1000L, pi);
                     }
                 }
             });

app/src/main/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClient.java

@@ -37,6 +37,7 @@
 import java.util.concurrent.TimeUnit;
 
 import okhttp3.Cache;
+import okhttp3.Call;
 import okhttp3.ConnectionPool;
 import okhttp3.Dns;
 import okhttp3.HttpUrl;
@@ -188,6 +189,25 @@ public void shutdown() {
 
     @Nullable
     public byte[] resolve(@NonNull byte[] dnsQuery) {
+        return resolve(dnsQuery, null);
+    }
+
+    /**
+     * Resolve a DNS query using DoH, reporting each wasted network attempt.
+     *
+     * @param dnsQuery         Raw DNS wire format query bytes
+     * @param onFailedAttempt  Invoked once per retryable failure (network error,
+     *                         server 5xx, or unusable response body), i.e. for
+     *                         every attempt that costs its full timeout budget.
+     *                         Not invoked for non-retryable client errors — the
+     *                         caller's own null handling still counts those once.
+     *                         Also not invoked when the request was canceled by
+     *                         our own {@link #getInstance} endpoint swap — that
+     *                         is not evidence the endpoint is unhealthy.
+     * @return DNS wire format response bytes, or null on failure
+     */
+    @Nullable
+    public byte[] resolve(@NonNull byte[] dnsQuery, @Nullable Runnable onFailedAttempt) {
         if (dnsQuery.length < 12) {
             Log.w(TAG, "DNS query too short: " + dnsQuery.length + " bytes");
             return null;
@@ -210,23 +230,27 @@ public byte[] resolve(@NonNull byte[] dnsQuery) {
                 Log.d(TAG, "DoH retry attempt " + attempt);
             }
 
+            Call call = client.newCall(request);
             try {
-                try (Response response = client.newCall(request).execute()) {
+                try (Response response = call.execute()) {
                     if (!response.isSuccessful()) {
                         Log.w(TAG, "DoH request failed with code: " + response.code());
                         if (response.code() < 500) return null; // Don't retry client errors
+                        reportFailedAttempt(onFailedAttempt);
                         continue;
                     }
 
                     ResponseBody responseBody = response.body();
                     if (responseBody == null) {
                         Log.w(TAG, "DoH response body is null");
+                        reportFailedAttempt(onFailedAttempt);
                         continue;
                     }
 
                     byte[] dnsResponse = responseBody.bytes();
                     if (dnsResponse.length < 12) {
                         Log.w(TAG, "DoH response too short: " + dnsResponse.length + " bytes");
+                        reportFailedAttempt(onFailedAttempt);
                         continue;
                     }
                     dnsResponse = finalizeResponse(dnsResponse, response, dnsQuery);
@@ -235,7 +259,16 @@ public byte[] resolve(@NonNull byte[] dnsQuery) {
                     return dnsResponse;
                 }
             } catch (IOException e) {
+                if (call.isCanceled()) {
+                    // Canceled by our own getInstance()/shutdown() swap (a DoH
+                    // endpoint change), not a real network failure. Counting
+                    // this would let an endpoint switch alone trip the circuit
+                    // breaker against a perfectly healthy new endpoint.
+                    Log.d(TAG, "DoH request canceled (client shutdown), not counted as a failure");
+                    return null;
+                }
                 Log.e(TAG, "DoH request failed: " + e.getMessage());
+                reportFailedAttempt(onFailedAttempt);
             } finally {
                 // Screen off: never leave an idle keep-alive socket behind — a
                 // server-side reset during doze would wake the radio. Cache
@@ -249,6 +282,14 @@ public byte[] resolve(@NonNull byte[] dnsQuery) {
         return null;
     }
 
+    private static void reportFailedAttempt(@Nullable Runnable onFailedAttempt) {
+        if (onFailedAttempt != null)
+            try {
+                onFailedAttempt.run();
+            } catch (Throwable ignored) {
+            }
+    }
+
     static byte[] normalizeTransactionId(byte[] dnsQuery) {
         byte[] normalized = Arrays.copyOf(dnsQuery, dnsQuery.length);
         normalized[0] = 0;

app/src/main/java/net/kollnig/missioncontrol/dns/DnsProxyServer.java

@@ -58,6 +58,9 @@ public class DnsProxyServer {
     private ServerSocket tcpServerSocket;
     private ExecutorService executor;
     private final AtomicInteger dohFailures = new AtomicInteger(0);
+    // Trips after this many consecutive failed network attempts (not queries):
+    // a single failing resolve burns up to three full timeout budgets, so a
+    // broken endpoint must stop being hammered within a few bad queries.
     private static final int CIRCUIT_BREAKER_THRESHOLD = 10;
     private static final long CIRCUIT_BREAKER_COOLDOWN_MS = 60_000;
     private static final int FALLBACK_DNS_TIMEOUT_MS = 5000;
@@ -262,10 +265,11 @@ private void handleQuery(byte[] queryData, InetAddress clientAddress, int client
 
             // Circuit breaker: skip DoH if we recently had too many failures
             boolean circuitOpen = System.currentTimeMillis() < circuitOpenUntil;
+            AtomicInteger queryFailedAttempts = new AtomicInteger(0);
             if (!circuitOpen) {
                 String endpoint = prefs.getString("doh_endpoint", BuildConfig.DEFAULT_DOH_ENDPOINT);
                 DnsOverHttpsClient dohClient = DnsOverHttpsClient.getInstance(context, endpoint);
-                responseData = dohClient.resolve(queryData);
+                responseData = dohClient.resolve(queryData, queryFailedAttempts::incrementAndGet);
             }
 
             if (responseData != null) {
@@ -279,8 +283,13 @@ private void handleQuery(byte[] queryData, InetAddress clientAddress, int client
                 Log.d(TAG, "DoH query successful, response sent to " + clientAddress + ":" + clientPort);
             } else {
                 if (!circuitOpen) {
-                    int failures = dohFailures.incrementAndGet();
-                    Log.w(TAG, "DoH query returned null response, failures=" + failures);
+                    // Each wasted attempt counts: one broken query can burn
+                    // three full timeout budgets, and waiting for ten whole
+                    // queries before tripping kept ~30s of hung work alive
+                    // per query on a dead network.
+                    int failures = dohFailures.addAndGet(Math.max(1, queryFailedAttempts.get()));
+                    Log.w(TAG, "DoH query returned null response after "
+                            + queryFailedAttempts.get() + " attempt(s), failures=" + failures);
 
                     if (failures >= CIRCUIT_BREAKER_THRESHOLD) {
                         circuitOpenUntil = System.currentTimeMillis() + CIRCUIT_BREAKER_COOLDOWN_MS;
@@ -410,10 +419,11 @@ private void handleTcpConnection(Socket client) {
             byte[] responseData = null;
 
             boolean circuitOpen = System.currentTimeMillis() < circuitOpenUntil;
+            AtomicInteger queryFailedAttempts = new AtomicInteger(0);
             if (!circuitOpen) {
                 String endpoint = prefs.getString("doh_endpoint", BuildConfig.DEFAULT_DOH_ENDPOINT);
                 DnsOverHttpsClient dohClient = DnsOverHttpsClient.getInstance(context, endpoint);
-                responseData = dohClient.resolve(queryData);
+                responseData = dohClient.resolve(queryData, queryFailedAttempts::incrementAndGet);
             }
 
             if (responseData != null) {
@@ -425,7 +435,8 @@ private void handleTcpConnection(Socket client) {
                 Log.d(TAG, "DoH TCP query successful");
             } else {
                 if (!circuitOpen) {
-                    int failures = dohFailures.incrementAndGet();
+                    // Same attempt-based accounting as the UDP path above.
+                    int failures = dohFailures.addAndGet(Math.max(1, queryFailedAttempts.get()));
                     if (failures >= CIRCUIT_BREAKER_THRESHOLD) {
                         circuitOpenUntil = System.currentTimeMillis() + CIRCUIT_BREAKER_COOLDOWN_MS;
                         dohFailures.set(0);

app/src/test/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClientTest.java

@@ -24,6 +24,7 @@
 import java.net.InetAddress;
 import java.util.Arrays;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import mockwebserver3.MockResponse;
 import mockwebserver3.MockWebServer;
@@ -130,6 +131,79 @@ public void resolveRetriesInvalidShortDnsResponse() {
         assertEquals(2, server.getRequestCount());
     }
 
+    // --- failed-attempt reporting (drives the DoH circuit breaker) --------
+
+    /**
+     * Every retryable failure — here three consecutive 5xx responses — must be
+     * reported individually: DnsProxyServer's circuit breaker counts wasted
+     * network attempts (each burns a full timeout budget), not queries.
+     */
+    @Test
+    public void resolveReportsEachRetryableServerError() {
+        server.enqueue(dnsResponse(503, new byte[0]));
+        server.enqueue(dnsResponse(503, new byte[0]));
+        server.enqueue(dnsResponse(503, new byte[0]));
+
+        AtomicInteger failures = new AtomicInteger(0);
+        assertNull(client().resolve(QUERY, failures::incrementAndGet));
+
+        assertEquals(3, failures.get());
+        assertEquals(3, server.getRequestCount());
+    }
+
+    /** A non-retryable client error is not reported as a wasted attempt. */
+    @Test
+    public void resolveDoesNotReportClientErrorsAsAttempts() {
+        server.enqueue(dnsResponse(400, new byte[0]));
+
+        AtomicInteger failures = new AtomicInteger(0);
+        assertNull(client().resolve(QUERY, failures::incrementAndGet));
+
+        assertEquals(0, failures.get());
+        assertEquals(1, server.getRequestCount());
+    }
+
+    /** An unusable 200 body is a wasted attempt and still retries to success. */
+    @Test
+    public void resolveReportsUnusableBodyThenRecovers() {
+        server.enqueue(dnsResponse(200, new byte[] { 1, 2, 3 }));
+        server.enqueue(dnsResponse(200, RESPONSE));
+
+        AtomicInteger failures = new AtomicInteger(0);
+        assertArrayEquals(RESPONSE, client().resolve(QUERY, failures::incrementAndGet));
+
+        assertEquals(1, failures.get());
+        assertEquals(2, server.getRequestCount());
+    }
+
+    /**
+     * A request canceled by our own client shutdown (e.g. a DoH endpoint
+     * switch via {@link DnsOverHttpsClient#getInstance}) must not be reported
+     * as a failed attempt — see issue #760: counting it would let switching
+     * endpoints alone trip the circuit breaker against the new, healthy one.
+     */
+    @Test
+    public void resolveDoesNotReportCanceledCallAsFailedAttempt() throws Exception {
+        server.enqueue(new MockResponse.Builder()
+                .code(200)
+                .addHeader("Content-Type", "application/dns-message")
+                .body(new Buffer().write(RESPONSE))
+                .headersDelay(2, TimeUnit.SECONDS)
+                .build());
+
+        DnsOverHttpsClient client = client();
+        AtomicInteger failures = new AtomicInteger(0);
+        Thread resolver = new Thread(() -> client.resolve(QUERY, failures::incrementAndGet));
+        resolver.start();
+
+        // Give the request time to actually be dispatched before shutting down.
+        server.takeRequest(1, TimeUnit.SECONDS);
+        client.shutdown();
+        resolver.join(5000);
+
+        assertEquals(0, failures.get());
+    }
+
     @Test
     public void normalizeTransactionIdUsesZeroWithoutMutatingQuery() {
         byte[] query = new byte[]{0x12, 0x34, 0x01, 0x00};