added timeouts to ban evasion lookups

craftattack/src/main/java/eu/mhsl/craftattack/spawn/craftattack/appliances/tooling/banEvasion/BanEvasion.java

@@ -32,9 +32,13 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.UUID;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.RejectedExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import java.util.function.Consumer;
 import java.util.logging.Level;
 
@@ -49,6 +53,9 @@ public class BanEvasion extends Appliance {
     static final String ADMIN_PERMISSION = "admin";
     private static final long TICKS_PER_MINUTE = 20L * 60;
     private static final long TICKS_PER_DAY = TICKS_PER_MINUTE * 60 * 24;
+    private static final Duration QUERY_TIMEOUT = Duration.ofSeconds(30);
+    private static final Duration BAN_LOOKUP_TIMEOUT = Duration.ofSeconds(5);
+    private static final Duration BAN_LOOKUP_BACKOFF = Duration.ofSeconds(30);
 
     @FunctionalInterface
     interface Loader<T> {
@@ -62,6 +69,7 @@ public class BanEvasion extends Appliance {
     private final OnlineNetworks onlineNetworks = new OnlineNetworks();
     private Database database;
     private ExecutorService worker;
+    private ExecutorService httpExecutor;
     private BukkitTask retentionTask;
     private BanStatusCache banStatus;
     private AdminNotifier notifier;
@@ -86,7 +94,12 @@ public class BanEvasion extends Appliance {
         this.window = Duration.ofDays(config.getInt("windowDays", 14));
         this.rejectionWindow = Duration.ofHours(config.getInt("rejectionWindowHours", 24));
         this.retentionMonths = config.getInt("retentionMonths", 16);
-        this.banStatus = new BanStatusCache(this::lookupBanState, Duration.ofMinutes(config.getInt("banCacheMinutes", 5)), Clock.systemUTC());
+        this.banStatus = new BanStatusCache(this::lookupBanState, Duration.ofMinutes(config.getInt("banCacheMinutes", 5)), BAN_LOOKUP_BACKOFF, Clock.systemUTC());
+        this.httpExecutor = Executors.newCachedThreadPool(runnable -> {
+            Thread thread = new Thread(runnable, "Spawn-BanEvasion-Http");
+            thread.setDaemon(true);
+            return thread;
+        });
         this.worker = Executors.newSingleThreadExecutor(runnable -> {
             Thread thread = new Thread(runnable, "Spawn-BanEvasion");
             thread.setDaemon(true);
@@ -103,6 +116,7 @@ public class BanEvasion extends Appliance {
         this.active = false;
         if(this.retentionTask != null) this.retentionTask.cancel();
         if(this.worker != null) this.worker.shutdownNow();
+        if(this.httpExecutor != null) this.httpExecutor.shutdownNow();
     }
 
     boolean isActive() {
@@ -121,7 +135,8 @@ public class BanEvasion extends Appliance {
      * Runs the query on the database thread and waits for it. Only for the worker thread.
      */
     <T> T query(Database.SqlQuery<T> query) {
-        return this.database.query(query).join();
+        // a dropped statement (full database queue) never completes, the worker must not wait for it forever
+        return this.database.query(query).orTimeout(QUERY_TIMEOUT.toSeconds(), TimeUnit.SECONDS).join();
     }
 
     /**
@@ -240,17 +255,32 @@ public class BanEvasion extends Appliance {
         );
     }
 
+    /**
+     * Asks the website with a time limit, so a hanging website cannot block the worker. A timed out request
+     * keeps running on its own daemon thread until the HTTP client gives up.
+     */
     private BanState lookupBanState(UUID uuid) {
         try {
+            return CompletableFuture.supplyAsync(() -> this.fetchBanState(uuid), this.httpExecutor)
+                .get(BAN_LOOKUP_TIMEOUT.toSeconds(), TimeUnit.SECONDS);
+        } catch(TimeoutException e) {
+            Main.logger().warning("BanEvasion: ban state of " + uuid + " unavailable: website did not answer in time");
+            return BanState.UNKNOWN;
+        } catch(ExecutionException e) {
+            Main.logger().warning("BanEvasion: ban state of " + uuid + " unavailable: " + e.getCause().getMessage());
+            return BanState.UNKNOWN;
+        } catch(InterruptedException e) {
+            Thread.currentThread().interrupt();
+            return BanState.UNKNOWN;
+        }
+    }
+
+    private BanState fetchBanState(UUID uuid) {
         ReqResp<WhitelistRepository.UserData> response = this.queryRepository(WhitelistRepository.class).getUserData(uuid);
         if(response.status() == HttpStatus.NOT_FOUND) return BanState.NOT_BANNED;
         if(response.status() != HttpStatus.OK || response.data() == null) return BanState.UNKNOWN;
         List<WhitelistRepository.UserData.Strike> strikes = Objects.requireNonNullElse(response.data().strikes(), List.of());
         return this.queryAppliance(Strikes.class).isBanned(strikes).isBanned() ? BanState.BANNED : BanState.NOT_BANNED;
-        } catch(RuntimeException e) {
-            Main.logger().warning("BanEvasion: ban state of " + uuid + " unavailable: " + e.getMessage());
-            return BanState.UNKNOWN;
-        }
     }
 
     private void deleteExpired() {

craftattack/src/main/java/eu/mhsl/craftattack/spawn/craftattack/appliances/tooling/banEvasion/BanStatusCache.java

@@ -11,8 +11,9 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
 
 /**
- * Ban states from the website, kept for a short time. UNKNOWN (API unreachable) is never cached.
+ * Ban states from the website, kept for a short time. UNKNOWN (API unreachable) is never cached,
- * The lookup blocks (HTTP), so never call this on the main or database thread.
+ * but after a failure the website is not asked again for the backoff period, so an unreachable website
+ * does not slow down every evaluation. The lookup blocks (HTTP), so never call this on the main or database thread.
  */
 final class BanStatusCache {
     private record Entry(BanState state, Instant fetchedAt) {
@@ -21,11 +22,14 @@ final class BanStatusCache {
     private final Map<UUID, Entry> entries = new ConcurrentHashMap<>();
     private final Function<UUID, BanState> lookup;
     private final Duration ttl;
+    private final Duration backoff;
     private final Clock clock;
+    private volatile Instant failingUntil = Instant.MIN;
 
-    BanStatusCache(Function<UUID, BanState> lookup, Duration ttl, Clock clock) {
+    BanStatusCache(Function<UUID, BanState> lookup, Duration ttl, Duration backoff, Clock clock) {
         this.lookup = lookup;
         this.ttl = ttl;
+        this.backoff = backoff;
         this.clock = clock;
     }
 
@@ -34,9 +38,15 @@ final class BanStatusCache {
         Entry entry = this.entries.get(uuid);
         if(entry != null && entry.fetchedAt().plus(this.ttl).isAfter(now)) return entry.state();
 
+        if(now.isBefore(this.failingUntil)) return BanState.UNKNOWN;
+
         BanState state = this.lookup.apply(uuid);
-        if(state == BanState.UNKNOWN) this.entries.remove(uuid);
+        if(state == BanState.UNKNOWN) {
-        else this.entries.put(uuid, new Entry(state, now));
+            this.entries.remove(uuid);
+            this.failingUntil = this.clock.instant().plus(this.backoff);
+        } else {
+            this.entries.put(uuid, new Entry(state, now));
+        }
         return state;
     }