Ensure oidc-rs-junit-tenant Port Reservation handles parallel test-runs

oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/jwt/PortReservation.java

@@ -3,6 +3,7 @@
 import java.io.IOException;
 import java.net.InetAddress;
 import java.net.ServerSocket;
+import java.security.SecureRandom;
 import javax.net.ServerSocketFactory;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -16,12 +17,17 @@ public class PortReservation {
     private static final int PORT_RANGE_START = 10000;
     private static final int PORT_RANGE_END = PORT_RANGE_MAX;
 
+    // seeded independently per JVM, so parallel test forks do not probe the same port sequence
+    private static final SecureRandom RANDOM = new SecureRandom();
+
     private final int portRangeStart;
     private final int portRangeEnd;
 
     private final String propertyName;
     private volatile int port = -1;
     private ServerSocket serverSocket;
+    // true when this instance picked the port itself; false when it was pinned via the property
+    private boolean portChosenByScan = false;
 
     public PortReservation(String portNames) {
         this(PORT_RANGE_START, PORT_RANGE_END, portNames);
@@ -53,29 +59,62 @@ public synchronized boolean start() {
             throw new IllegalArgumentException(
                     "Port range end must not be larger than " + PORT_RANGE_MAX + ".");
         }
+        if (serverSocket != null) {
+            return true; // already holding a reservation
+        }
         // check if the property already exists, if so it must be free
         String property = System.getProperty(propertyName);
         if (property != null) {
-            if (reserve(Integer.parseInt(property), true)) {
+            int preconfiguredPort = Integer.parseInt(property);
+            // distinguish a port this instance picked earlier from one pinned externally
+            boolean ownPort = portChosenByScan && preconfiguredPort == this.port;
+            if (reserve(preconfiguredPort, true)) {
+                portChosenByScan = ownPort;
                 log.warn("Reserved previously configured port " + property);
                 return true;
-            } else {
+            }
+            if (!ownPort) {
                 throw new IllegalArgumentException("Preconfigured port " + property + " is not free");
             }
+            // the port this reservation picked earlier has been taken by another process;
+            // abandon it and scan for a new one
+            System.clearProperty(propertyName);
+            this.port = -1;
         }
-        // systematically try ports in range
-        // starting at 'random' offset
-        int portRange = portRangeEnd - portRangeStart + 1;
+        scan();
+        return true;
+    }
 
-        int offset =
-                (propertyName.hashCode() + (int) System.currentTimeMillis())
-                        % portRange; // more or less random per port name
+    /**
+     * Abandon the current reservation and reserve a fresh port from the range. Intended for recovery
+     * when another process binds the port between {@link #stop()} and the consumer's own bind
+     * attempt.
+     *
+     * @throws IllegalStateException if the port was preconfigured externally and therefore cannot be
+     *     changed
+     */
+    public synchronized void rescan() {
+        if (!portChosenByScan && System.getProperty(propertyName) != null) {
+            throw new IllegalStateException(
+                    "Preconfigured port " + System.getProperty(propertyName) + " cannot be rescanned");
+        }
+        stop();
+        System.clearProperty(propertyName);
+        this.port = -1;
+        scan();
+    }
+
+    private void scan() {
+        // systematically try ports in range, starting at a random offset
+        int portRange = portRangeEnd - portRangeStart + 1;
+        int offset = RANDOM.nextInt(portRange);
 
         for (int i = 0; i < portRange; i++) {
-            int candidatePort = portRangeStart + (offset + portRange) % portRange;
+            int candidatePort = portRangeStart + (offset + i) % portRange;
             if (reserve(candidatePort, false)) {
+                portChosenByScan = true;
                 log.warn("Reserved newly configured port " + candidatePort);
-                return true;
+                return;
             }
         }
         throw new IllegalArgumentException("Unable to reserve free port");
@@ -95,35 +134,34 @@ public synchronized void stop() {
     }
 
     private boolean reserve(int candidatePort, boolean retry) {
-        // Retry on failure, 10 times.
-        for (int i = 0; i < 10; i++) {
+        // While scanning, an occupied port is skipped immediately so the next candidate can be
+        // tried; only a specific (preconfigured or previously reserved) port is worth waiting for.
+        int attempts = retry ? 10 : 1;
+        for (int i = 0; i < attempts; i++) {
 
             if (i > 0) {
                 log.debug("Waiting 1 second before try reserve port {}.", candidatePort);
                 try {
-                    wait(1000); // Wait 1 second
+                    // Object.wait releases the monitor while sleeping, so other threads are not
+                    // blocked for the full retry duration; a stray notify only shortens the wait
+                    wait(1000);
                 } catch (InterruptedException e) {
                     Thread.currentThread().interrupt();
                     break;
                 }
             }
 
-            try {
-                ServerSocket result = capturePort(candidatePort);
-                if (result != null) {
-                    reserved(candidatePort, result);
-                    return true;
-                }
-            } catch (Exception e) {
-                if (!retry) {
-                    return false;
-                }
+            ServerSocket result = capturePort(candidatePort);
+            if (result != null) {
+                reserved(candidatePort, result);
+                return true;
             }
         }
         return false;
     }
 
     private void reserved(int port, ServerSocket serverSocket) {
+        stop(); // release any socket a concurrent caller reserved while this thread waited
         this.port = port;
         this.serverSocket = serverSocket;
 

oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/tenant/TenantAnnotationTokenFactory.java

@@ -1,6 +1,7 @@
 package org.entur.auth.junit.tenant;
 
 import com.github.tomakehurst.wiremock.client.WireMock;
+import com.github.tomakehurst.wiremock.common.FatalStartupException;
 import java.lang.annotation.Annotation;
 import java.time.Instant;
 import java.time.ZonedDateTime;
@@ -42,6 +43,8 @@
  */
 @Slf4j
 public class TenantAnnotationTokenFactory implements AutoCloseable {
+    private static final int MAX_BIND_ATTEMPTS = 5;
+
     private final Provider provider;
     private final PortReservation portReservation;
     private JwtTokenFactory jwtTokenFactory;
@@ -60,8 +63,44 @@ public TenantAnnotationTokenFactory(
         this.portReservation = portReservation;
 
         /* Ensure the WireMock server is running, reserving the port if needed. */
-        portReservation.stop();
-        setServer(new WireMockAuthenticationServer(portReservation.getPort()));
+        setServer(startServer(portReservation));
+    }
+
+    /**
+     * Start a WireMock server on the reserved port. The reservation socket is released immediately
+     * before binding; if another process grabs the port in that window, a fresh port is reserved and
+     * the bind is retried instead of failing the whole test run.
+     */
+    private static WireMockAuthenticationServer startServer(final PortReservation portReservation) {
+        if (portReservation.getPort() < 0) {
+            portReservation.start();
+        }
+
+        FatalStartupException lastFailure = null;
+        for (int attempt = 1; ; attempt++) {
+            if (attempt > MAX_BIND_ATTEMPTS) {
+                throw lastFailure;
+            }
+            if (lastFailure != null) {
+                // the previous bind failed; pick a fresh port before retrying
+                try {
+                    portReservation.rescan();
+                } catch (IllegalStateException pinnedPort) {
+                    pinnedPort.initCause(lastFailure);
+                    throw pinnedPort;
+                }
+            }
+            portReservation.stop();
+            try {
+                return new WireMockAuthenticationServer(portReservation.getPort());
+            } catch (FatalStartupException e) {
+                lastFailure = e;
+                log.warn(
+                        "Failed to bind mock server to reserved port {}, rescanning for a new port",
+                        portReservation.getPort(),
+                        e);
+            }
+        }
     }
 
     /**

oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/tenant/TenantJsonWebToken.java

@@ -65,9 +65,17 @@ public class TenantJsonWebToken implements ParameterResolver, BeforeAllCallback
     /** Manages reservation of the network port for the WireMock authentication server. */
     private static PortReservation portReservation;
 
-    /** Construct a new TenantJsonWebToken and initializes the reserved port */
+    /**
+     * Construct a new TenantJsonWebToken, reserving the port and initializing the token factory and
+     * WireMock server.
+     *
+     * <p>This happens during extension construction (before any lifecycle callback) so the final port
+     * is published to the {@link #MOCKAUTHSERVER_PORT_NAME} system property before {@code
+     * SpringExtension} loads the context. This keeps the Spring context wired to the same port the
+     * mock server listens on, even when the port has to be retried because the original was stolen.
+     */
     public TenantJsonWebToken() {
-        setupPortReservation();
+        setupTokenFactory();
     }
 
     /**

oidc-rs-junit-tenant/src/test/java/org/entur/auth/junit/jwt/PortReservationTest.java

@@ -0,0 +1,135 @@
+package org.entur.auth.junit.jwt;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.ServerSocket;
+import org.junit.jupiter.api.Test;
+
+class PortReservationTest {
+
+    @Test
+    void scanSkipsOccupiedPortsAndAdvancesToNextCandidate() throws IOException {
+        String propertyName = "PortReservationTest.scan";
+        int base = findConsecutiveFreePorts(3);
+        try (ServerSocket occupiedFirst = bind(base);
+                ServerSocket occupiedSecond = bind(base + 1)) {
+            PortReservation reservation = new PortReservation(base, base + 2, propertyName);
+            try {
+                assertTrue(reservation.start());
+                assertEquals(base + 2, reservation.getPort());
+                assertEquals(Integer.toString(base + 2), System.getProperty(propertyName));
+            } finally {
+                reservation.stop();
+                System.clearProperty(propertyName);
+            }
+        }
+    }
+
+    @Test
+    void rescanReservesNewPortWhenPreviousPortIsStolen() throws IOException {
+        String propertyName = "PortReservationTest.rescan";
+        PortReservation reservation = new PortReservation(propertyName);
+        try {
+            reservation.start();
+            int stolenPort = reservation.getPort();
+
+            reservation.stop();
+            try (ServerSocket thief = bind(stolenPort)) {
+                reservation.rescan();
+
+                assertTrue(reservation.getPort() > 0);
+                assertNotEquals(stolenPort, reservation.getPort());
+                assertEquals(Integer.toString(reservation.getPort()), System.getProperty(propertyName));
+            }
+        } finally {
+            reservation.stop();
+            System.clearProperty(propertyName);
+        }
+    }
+
+    /**
+     * Simulates another process stealing the reserved port in the window after {@link
+     * PortReservation#stop()}. The stale port stays pinned until recovery is requested, but
+     * re-running {@link PortReservation#start()} (as {@code TenantAnnotationTokenFactory#close()}
+     * does) must abandon the stolen port and reserve a fresh one instead of failing.
+     *
+     * <p>Reproduces the upstream oidc-lib bug where start() instead throws {@code
+     * IllegalArgumentException: Preconfigured port ... is not free}, pinning the dead port forever.
+     *
+     * <p>Slow by design: start() retries the previously reserved port for ~10 seconds before giving
+     * it up, in case the thief is short-lived.
+     */
+    @Test
+    void restartReservesNewPortWhenPreviousPortIsStolen() throws IOException {
+        String propertyName = "PortReservationTest.restart";
+        PortReservation reservation = new PortReservation(propertyName);
+        try {
+            reservation.start();
+            int stolenPort = reservation.getPort();
+
+            reservation.stop();
+            try (ServerSocket thief = bind(stolenPort)) {
+                // the stale reservation still reports the stolen port...
+                assertEquals(stolenPort, reservation.getPort());
+
+                // ...but restarting recovers by abandoning it and scanning for a new one
+                assertTrue(reservation.start());
+                assertTrue(reservation.getPort() > 0);
+                assertNotEquals(stolenPort, reservation.getPort());
+                assertEquals(Integer.toString(reservation.getPort()), System.getProperty(propertyName));
+            }
+        } finally {
+            reservation.stop();
+            System.clearProperty(propertyName);
+        }
+    }
+
+    @Test
+    void externallyPreconfiguredPortIsReusedAndCannotBeRescanned() throws IOException {
+        String propertyName = "PortReservationTest.preconfigured";
+        int freePort;
+        try (ServerSocket probe = bind(0)) {
+            freePort = probe.getLocalPort();
+        }
+        System.setProperty(propertyName, Integer.toString(freePort));
+        PortReservation reservation = new PortReservation(propertyName);
+        try {
+            assertTrue(reservation.start());
+            assertEquals(freePort, reservation.getPort());
+            assertThrows(IllegalStateException.class, reservation::rescan);
+        } finally {
+            reservation.stop();
+            System.clearProperty(propertyName);
+        }
+    }
+
+    private static ServerSocket bind(int port) throws IOException {
+        return new ServerSocket(port, 1, InetAddress.getByName("localhost"));
+    }
+
+    private static int findConsecutiveFreePorts(int count) throws IOException {
+        for (int base = 24000; base < 64000; base += count) {
+            ServerSocket[] sockets = new ServerSocket[count];
+            try {
+                for (int i = 0; i < count; i++) {
+                    sockets[i] = bind(base + i);
+                }
+                return base;
+            } catch (IOException e) {
+                // try the next block of ports
+            } finally {
+                for (ServerSocket socket : sockets) {
+                    if (socket != null) {
+                        socket.close();
+                    }
+                }
+            }
+        }
+        throw new IOException("Unable to find " + count + " consecutive free ports");
+    }
+}