From a0150bf71e8e4d630fe55ae04482951a5ca3e179 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Sat, 10 Oct 2026 17:10:29 +0000 Subject: [PATCH 1/4] SOLR-18531: Hold a stopped JettySolrRunner port until the next start on it JettySolrRunner released its port on stop and rebound the same pinned port on start, so a process outside the test JVM could take the port in the gap and the restart failed with BindException once the holder outlived the bind retry budget (seen in LeaderElectionIntegrationTest on CI). stop() now binds a reservation socket to the runner port and keeps it in a static map; start() releases the reservation for the port it is about to bind. The map is static so a replacement runner created for the same port sees the reservation. MiniSolrCloudCluster.shutdown() releases its runners reservations, since they will not start again. Adds a deterministic test: after stop(), a foreign bind to the port is refused, and a restart on the same port succeeds. The test uses only the existing public API, so it also runs against the pre-fix framework, where the foreign bind succeeds. --- ...LR-18531-jetty-runner-port-reservation.yml | 10 +++ .../solr/cloud/MiniSolrCloudCluster.java | 11 +++- .../apache/solr/embedded/JettySolrRunner.java | 62 +++++++++++++++++++ .../solr/embedded/TestJettySolrRunner.java | 43 +++++++++++++ 4 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 changelog/unreleased/SOLR-18531-jetty-runner-port-reservation.yml diff --git a/changelog/unreleased/SOLR-18531-jetty-runner-port-reservation.yml b/changelog/unreleased/SOLR-18531-jetty-runner-port-reservation.yml new file mode 100644 index 000000000000..c29583ed6c9c --- /dev/null +++ b/changelog/unreleased/SOLR-18531-jetty-runner-port-reservation.yml @@ -0,0 +1,10 @@ +title: >- + JettySolrRunner in the test framework now holds a stopped runner's port until the next + start on that port, so a restart can no longer fail with BindException when another + process takes the port during the restart gap +type: fixed +authors: + - name: Nick Shanin +links: + - name: SOLR-18531 + url: https://issues.apache.org/jira/browse/SOLR-18531 diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java index f9acdbfc0bf5..c5b9acfb0476 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java @@ -624,8 +624,9 @@ public void shutdown() throws Exception { }); solrClientByCollection.clear(); - List> shutdowns = new ArrayList<>(jettys.size()); - for (final JettySolrRunner jetty : jettys) { + List stoppedJettys = new ArrayList<>(jettys); + List> shutdowns = new ArrayList<>(stoppedJettys.size()); + for (final JettySolrRunner jetty : stoppedJettys) { shutdowns.add(() -> stopJettySolrRunner(jetty)); } jettys.clear(); @@ -635,6 +636,12 @@ public void shutdown() throws Exception { ExecutorUtil.shutdownAndAwaitTermination(executorCloser); Exception shutdownError = checkForExceptions("Error shutting down MiniSolrCloudCluster", futures); + // A stopped runner holds its port in reserve for a restart; this cluster will not + // restart its runners, so release the reservations instead of keeping the ports + // until the JVM exits. + for (final JettySolrRunner jetty : stoppedJettys) { + jetty.releasePortReservation(); + } if (shutdownError != null) { throw shutdownError; } diff --git a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java index ff8253853ff1..96bc0a86916c 100644 --- a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java +++ b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java @@ -26,7 +26,9 @@ import java.io.PrintStream; import java.lang.invoke.MethodHandles; import java.net.BindException; +import java.net.InetSocketAddress; import java.net.MalformedURLException; +import java.net.ServerSocket; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; @@ -41,6 +43,7 @@ import java.util.Properties; import java.util.Random; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import org.apache.solr.SolrBackend; @@ -109,6 +112,13 @@ public class JettySolrRunner implements SolrBackend { // NOTE: needs to be larger than SolrHttpClient.threadPoolSweeperMaxIdleTime private static final int THREAD_POOL_MAX_IDLE_TIME_MS = 260000; + // Ports held on behalf of stopped runners, so that no other process can take a stopped + // runner's port before a runner starts on it again. Static because a replacement runner + // can be created for the same port (SSLMigrationTest does this); the reservation has to + // be visible across instances. An entry is released when a runner starts on the port, and + // by MiniSolrCloudCluster.shutdown() for the runners of a cluster that is gone. + private static final Map RESERVED_PORTS = new ConcurrentHashMap<>(); + private Server server; private volatile ServletHolder solrServlet; @@ -497,6 +507,10 @@ public synchronized void start(boolean reusePort) throws Exception { int port = reusePort && jettyPort != -1 ? jettyPort : this.config.port; log.info("Start Jetty (configured port={}, binding port={})", this.config.port, port); + // Release the reservation on the port we are about to bind, whether this runner's + // stop() left it or another runner was stopped on the same port. + releasePortReservation(port); + // if started before, make a new server if (startedBefore) { init(port); @@ -660,6 +674,10 @@ public synchronized void stop() throws Exception { } } while (!server.isStopped()); + // Hold the port until the next start on it, so that another process cannot take + // it in the gap and make a restart fail with BindException. + reserveJettyPort(); + } finally { if (prevContext != null) { MDC.setContextMap(prevContext); @@ -669,6 +687,50 @@ public synchronized void stop() throws Exception { } } + /** + * Holds this runner's port after stop, by binding a socket to it, until a runner starts on the + * port again or the reservation is released. If the port cannot be held, a restart behaves as it + * did without reservations: it binds the port if it is still free, and the usual bind retry + * applies if it is not. + */ + private void reserveJettyPort() { + if (jettyPort <= 0) { + return; + } + ServerSocket socket = null; + try { + socket = new ServerSocket(); + socket.setReuseAddress(false); + socket.bind(new InetSocketAddress("127.0.0.1", jettyPort)); + } catch (IOException e) { + log.warn( + "Could not reserve port {} after stop; a restart on this port is not protected", + jettyPort, + e); + IOUtils.closeQuietly(socket); + return; + } + if (RESERVED_PORTS.putIfAbsent(jettyPort, socket) != null) { + IOUtils.closeQuietly(socket); + } + } + + /** + * Releases the reservation held on this runner's port, if any. Called by + * MiniSolrCloudCluster.shutdown(), whose runners will not start again; a runner that starts on + * the port releases the reservation itself. + */ + public void releasePortReservation() { + releasePortReservation(jettyPort); + } + + private static void releasePortReservation(int port) { + ServerSocket socket = RESERVED_PORTS.remove(port); + if (socket != null) { + IOUtils.closeQuietly(socket); + } + } + public void outputMetrics(PrintStream out) throws IOException { if (getCoreContainer() != null) { SolrMetricManager metricsManager = getCoreContainer().getMetricManager(); diff --git a/solr/test-framework/src/test/org/apache/solr/embedded/TestJettySolrRunner.java b/solr/test-framework/src/test/org/apache/solr/embedded/TestJettySolrRunner.java index a874787c0ead..1d4cd2835e22 100644 --- a/solr/test-framework/src/test/org/apache/solr/embedded/TestJettySolrRunner.java +++ b/solr/test-framework/src/test/org/apache/solr/embedded/TestJettySolrRunner.java @@ -18,6 +18,8 @@ import java.io.IOException; import java.net.BindException; +import java.net.InetSocketAddress; +import java.net.ServerSocket; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -113,4 +115,45 @@ public synchronized Throwable getCause() { result = jetty.lookForBindException(test); assertEquals(result, be); } + + @Test + public void testStoppedRunnerKeepsItsPortUntilRestart() throws Exception { + Path solrHome = createTempDir(); + Files.write( + solrHome.resolve("solr.xml"), + MiniSolrCloudCluster.DEFAULT_CLOUD_SOLR_XML.getBytes(Charset.defaultCharset())); + + JettyConfig config = JettyConfig.builder().build(); + JettySolrRunner runner = new JettySolrRunner(solrHome.toString(), config); + + boolean running = false; + try { + runner.start(); + running = true; + int port = runner.getLocalPort(); + + runner.stop(); + running = false; + + // The framework holds the stopped runner's port, so a foreign process cannot take + // it during the restart gap. This uses only the long-standing public API, so the + // test also runs against the pre-fix framework, where the bind below succeeds. + try (ServerSocket foreign = new ServerSocket()) { + foreign.setReuseAddress(false); + foreign.bind(new InetSocketAddress("127.0.0.1", port)); + fail("the stopped runner's port should still be reserved"); + } catch (BindException expected) { + // the reservation is doing its job + } + + // Restarting on the same port still works. + runner.start(); + running = true; + assertEquals(port, runner.getLocalPort()); + } finally { + if (running) { + runner.stop(); + } + } + } } From 5ba914ca745933251958412a4ccb226ecbaf0ad8 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Sat, 10 Oct 2026 19:25:56 +0000 Subject: [PATCH 2/4] SOLR-18531 Release the port reservation when a runner is closed, and correct the release rules Review of the first version found that a runner stopped and removed from its cluster is never released at cluster shutdown, and that close() only called stop(), which reserves the port again. close() now releases the reservation after stopping: close is the end of a runner life, while stop() stays one half of the stop and restart cycle the reservation protects. The reservation documentation now states the three release routes exactly (a start on the port, cluster shutdown for runners still in the cluster, or close) and that anything else holds the port until the JVM exits, including a runner restarted on a different port than the one it reserved. The changelog title no longer claims a restart cannot fail with BindException; it states what the change does. The new test also covers the close() release. --- ...LR-18531-jetty-runner-port-reservation.yml | 5 ++- .../solr/cloud/MiniSolrCloudCluster.java | 4 ++- .../apache/solr/embedded/JettySolrRunner.java | 32 +++++++++++++------ .../solr/embedded/TestJettySolrRunner.java | 14 ++++++++ 4 files changed, 42 insertions(+), 13 deletions(-) diff --git a/changelog/unreleased/SOLR-18531-jetty-runner-port-reservation.yml b/changelog/unreleased/SOLR-18531-jetty-runner-port-reservation.yml index c29583ed6c9c..75d6f9657724 100644 --- a/changelog/unreleased/SOLR-18531-jetty-runner-port-reservation.yml +++ b/changelog/unreleased/SOLR-18531-jetty-runner-port-reservation.yml @@ -1,7 +1,6 @@ title: >- - JettySolrRunner in the test framework now holds a stopped runner's port until the next - start on that port, so a restart can no longer fail with BindException when another - process takes the port during the restart gap + JettySolrRunner in the test framework now holds a stopped runner's port in reserve until + the next start on that port type: fixed authors: - name: Nick Shanin diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java index c5b9acfb0476..75429b1ccc37 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java @@ -638,7 +638,9 @@ public void shutdown() throws Exception { checkForExceptions("Error shutting down MiniSolrCloudCluster", futures); // A stopped runner holds its port in reserve for a restart; this cluster will not // restart its runners, so release the reservations instead of keeping the ports - // until the JVM exits. + // until the JVM exits. This covers only the runners still in the cluster: a runner + // removed earlier by stopJettySolrRunner is not in this list, and its reservation + // stays in place until that runner is closed or the JVM exits. for (final JettySolrRunner jetty : stoppedJettys) { jetty.releasePortReservation(); } diff --git a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java index 96bc0a86916c..624d56b3a74e 100644 --- a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java +++ b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java @@ -115,8 +115,13 @@ public class JettySolrRunner implements SolrBackend { // Ports held on behalf of stopped runners, so that no other process can take a stopped // runner's port before a runner starts on it again. Static because a replacement runner // can be created for the same port (SSLMigrationTest does this); the reservation has to - // be visible across instances. An entry is released when a runner starts on the port, and - // by MiniSolrCloudCluster.shutdown() for the runners of a cluster that is gone. + // be visible across instances. An entry is released in exactly three ways: a runner + // starts on the port; MiniSolrCloudCluster.shutdown() releases the runners still in the + // cluster at shutdown; or the runner holding the entry is closed. In every other case + // the entry is held until the JVM exits. That includes a runner stopped and removed + // from its cluster and never closed, and a runner restarted on a different port than + // the one it reserved: a release only ever names the port a runner starts on or the + // port it currently holds, never a port it held earlier. private static final Map RESERVED_PORTS = new ConcurrentHashMap<>(); private Server server; @@ -688,10 +693,10 @@ public synchronized void stop() throws Exception { } /** - * Holds this runner's port after stop, by binding a socket to it, until a runner starts on the - * port again or the reservation is released. If the port cannot be held, a restart behaves as it - * did without reservations: it binds the port if it is still free, and the usual bind retry - * applies if it is not. + * Holds this runner's port after stop, by binding a socket to it, until the reservation is + * released; see {@link #RESERVED_PORTS} for the exact release rules. If the port cannot be held, + * a restart behaves as it did without reservations: it binds the port if it is still free, and + * the usual bind retry applies if it is not. */ private void reserveJettyPort() { if (jettyPort <= 0) { @@ -716,9 +721,13 @@ private void reserveJettyPort() { } /** - * Releases the reservation held on this runner's port, if any. Called by - * MiniSolrCloudCluster.shutdown(), whose runners will not start again; a runner that starts on - * the port releases the reservation itself. + * Releases the reservation held on this runner's current port, if any. Called by + * MiniSolrCloudCluster.shutdown() for the runners still in the cluster at shutdown, whose runners + * will not start again, and by {@link #close()}, after which this runner will not start again + * either. A runner removed from its cluster by {@code stopJettySolrRunner} is not covered by the + * shutdown call: if it is never closed, its reservation stays in place until a runner starts on + * the port or the JVM exits. The same applies to the old port of a runner restarted on a + * different port, because this method names only the port the runner currently holds. */ public void releasePortReservation() { releasePortReservation(jettyPort); @@ -949,5 +958,10 @@ public void close() { } catch (Exception e) { log.error(e.toString(), e); // nowarn } + // close() is the end of this runner's life, unlike stop(), which is one half of the + // stop and restart cycle the reservation protects. Give the port back instead of + // leaving stop()'s reservation in place until the JVM exits. Runners still in a + // MiniSolrCloudCluster get the same release from its shutdown(). + releasePortReservation(); } } diff --git a/solr/test-framework/src/test/org/apache/solr/embedded/TestJettySolrRunner.java b/solr/test-framework/src/test/org/apache/solr/embedded/TestJettySolrRunner.java index 1d4cd2835e22..ef326c47b441 100644 --- a/solr/test-framework/src/test/org/apache/solr/embedded/TestJettySolrRunner.java +++ b/solr/test-framework/src/test/org/apache/solr/embedded/TestJettySolrRunner.java @@ -150,6 +150,20 @@ public void testStoppedRunnerKeepsItsPortUntilRestart() throws Exception { runner.start(); running = true; assertEquals(port, runner.getLocalPort()); + + // Closing a stopped runner gives the port back. close() ends the runner's life, + // so nothing keeps the reservation afterwards, unlike stop(), which is one half + // of the stop and restart cycle. + runner.stop(); + running = false; + runner.close(); + try (ServerSocket foreign = new ServerSocket()) { + foreign.setReuseAddress(false); + foreign.bind(new InetSocketAddress("127.0.0.1", port)); + // the bind succeeded, so the reservation is gone + } catch (BindException e) { + fail("close() should release the stopped runner's port reservation"); + } } finally { if (running) { runner.stop(); From 351914f52c99180f0582d45c5bea1bd800194d29 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Sat, 10 Oct 2026 20:36:32 +0000 Subject: [PATCH 3/4] SOLR-18531 Commit the served-traffic reservation test, and fix the reservation bind it exposed The new testStoppedRunnerThatServedTrafficKeepsItsPortUntilRestart serves a real request through its own client before stopping the runner, then checks the port stays reserved against foreign binds with and without address reuse, and that a restart on the same port serves again. Run at the gate seed, that test showed the reservation itself could fail: connections the stopped server closed can leave the port in TIME_WAIT, and the reservation socket bound without address reuse, so it could not bind over them and the port went unprotected. Jetty connectors bind with reuse set. The reservation socket now binds with address reuse as well; the listening socket it holds still refuses every later bind, so the protection is unchanged. --- .../apache/solr/embedded/JettySolrRunner.java | 7 ++- .../solr/embedded/TestJettySolrRunner.java | 60 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java index 624d56b3a74e..2821ffa828fe 100644 --- a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java +++ b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java @@ -705,7 +705,12 @@ private void reserveJettyPort() { ServerSocket socket = null; try { socket = new ServerSocket(); - socket.setReuseAddress(false); + // Address reuse lets this bind succeed over connection sockets the stopped server + // left in TIME_WAIT (its connectors bind with reuse set as well); without it, a + // runner that served traffic before stopping could not reserve its port at all. + // The listening socket held here still refuses every later bind, with or without + // reuse, so the port stays protected. + socket.setReuseAddress(true); socket.bind(new InetSocketAddress("127.0.0.1", jettyPort)); } catch (IOException e) { log.warn( diff --git a/solr/test-framework/src/test/org/apache/solr/embedded/TestJettySolrRunner.java b/solr/test-framework/src/test/org/apache/solr/embedded/TestJettySolrRunner.java index ef326c47b441..ce5707c0d134 100644 --- a/solr/test-framework/src/test/org/apache/solr/embedded/TestJettySolrRunner.java +++ b/solr/test-framework/src/test/org/apache/solr/embedded/TestJettySolrRunner.java @@ -27,7 +27,9 @@ import java.util.Properties; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.jetty.HttpJettySolrClient; import org.apache.solr.client.solrj.request.CoreAdminRequest; +import org.apache.solr.client.solrj.request.CoresApi; import org.apache.solr.cloud.MiniSolrCloudCluster; import org.junit.Test; @@ -170,4 +172,62 @@ public void testStoppedRunnerKeepsItsPortUntilRestart() throws Exception { } } } + + @Test + public void testStoppedRunnerThatServedTrafficKeepsItsPortUntilRestart() throws Exception { + Path solrHome = createTempDir(); + Files.write( + solrHome.resolve("solr.xml"), + MiniSolrCloudCluster.DEFAULT_CLOUD_SOLR_XML.getBytes(Charset.defaultCharset())); + + JettyConfig config = JettyConfig.builder().build(); + JettySolrRunner runner = new JettySolrRunner(solrHome.toString(), config); + + boolean running = false; + try { + runner.start(); + running = true; + int port = runner.getLocalPort(); + + // Serve a real request before the stop, through a client of this test rather than + // the runner's own, which stop() closes. Connections a node served can leave the + // port in TIME_WAIT once it stops, which the reservation bind has to tolerate (it + // binds with address reuse, like the server connectors) while still holding the + // port. The plain stop in the test above never serves traffic, so it cannot reach + // this case. + try (HttpJettySolrClient client = + new HttpJettySolrClient.Builder(runner.getBaseUrl().toString()).build()) { + new CoresApi.GetAllCoreStatus().process(client); + } + + runner.stop(); + running = false; + + // A foreign bind with address reuse could take over connection sockets lingering + // in TIME_WAIT, but it must still fail against the socket the framework itself + // holds on the port, with or without reuse. + for (boolean reuse : new boolean[] {false, true}) { + try (ServerSocket foreign = new ServerSocket()) { + foreign.setReuseAddress(reuse); + foreign.bind(new InetSocketAddress("127.0.0.1", port)); + fail("the stopped runner's port should still be reserved (reuseAddress=" + reuse + ")"); + } catch (BindException expected) { + // the reservation is doing its job + } + } + + // Restarting on the same port still works, and the runner serves again. + runner.start(); + running = true; + assertEquals(port, runner.getLocalPort()); + try (HttpJettySolrClient client = + new HttpJettySolrClient.Builder(runner.getBaseUrl().toString()).build()) { + new CoresApi.GetAllCoreStatus().process(client); + } + } finally { + if (running) { + runner.stop(); + } + } + } } From c8a67c7267d00e33ad8323c683c040b539989b0d Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Sat, 10 Oct 2026 22:18:28 +0000 Subject: [PATCH 4/4] SOLR-18531: Skip a duplicate port reservation on a repeated stop A second stop() on a runner whose port is already reserved (close() after stop(), or a stop followed by cluster shutdown) tried to bind the port again, failed against the held socket, and logged a warning claiming a restart on the port is not protected when it is. Return early when a reservation for the port is already held. Also qualify the bind comment: the held socket refusing later binds is verified on Linux; address-reuse bind semantics differ on Windows. --- .../java/org/apache/solr/embedded/JettySolrRunner.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java index 2821ffa828fe..9b3b0db7cbc0 100644 --- a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java +++ b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java @@ -702,6 +702,13 @@ private void reserveJettyPort() { if (jettyPort <= 0) { return; } + if (RESERVED_PORTS.containsKey(jettyPort)) { + // A reservation for this port is already held: a second stop() on this runner + // (close() after stop(), or a stop followed by cluster shutdown), or another + // runner stopped on the same port. Binding again would fail against the held + // socket and log a warning claiming the port is unprotected when it is not. + return; + } ServerSocket socket = null; try { socket = new ServerSocket(); @@ -709,7 +716,8 @@ private void reserveJettyPort() { // left in TIME_WAIT (its connectors bind with reuse set as well); without it, a // runner that served traffic before stopping could not reserve its port at all. // The listening socket held here still refuses every later bind, with or without - // reuse, so the port stays protected. + // reuse, on Linux; address-reuse bind semantics differ on Windows, where the + // same exclusion is not verified. socket.setReuseAddress(true); socket.bind(new InetSocketAddress("127.0.0.1", jettyPort)); } catch (IOException e) {