core/src/test/java/com/linecorp/armeria/server/PortBasedVirtualHostTest.java (1)

241-331: Add assertions for /bar routing in virtualHostWithServerPort.
Right now /bar responses are computed but never asserted, so misrouting could slip through.

✅ Proposed test assertion

             // Check /bar on both ports
             final AggregatedHttpResponse resp1Bar = client1.get("/bar").aggregate().join();
             final AggregatedHttpResponse resp2Bar = client2.get("/bar").aggregate().join();

             // One port should serve /foo with OK, the other with NOT_FOUND
             final boolean port1ServesFoo = resp1Foo.status() == HttpStatus.OK;
             final boolean port2ServesFoo = resp2Foo.status() == HttpStatus.OK;
             assertThat(port1ServesFoo || port2ServesFoo)
                     .as("At least one port should serve /foo, but got: port1=%s, port2=%s",
                         resp1Foo.status(), resp2Foo.status())
                     .isTrue();
+            // One port should serve /bar with OK as well.
+            final boolean port1ServesBar = resp1Bar.status() == HttpStatus.OK;
+            final boolean port2ServesBar = resp2Bar.status() == HttpStatus.OK;
+            assertThat(port1ServesBar || port2ServesBar)
+                    .as("At least one port should serve /bar, but got: port1=%s, port2=%s",
+                        resp1Bar.status(), resp2Bar.status())
+                    .isTrue();

core/src/main/java/com/linecorp/armeria/server/VirtualHost.java (1)

316-341: Consider documenting the dynamic port resolution behavior.

The defaultHostname() and hostnamePattern() methods now return different values depending on whether the server has bound to its ports. Before binding, serverPort.localAddress() returns null, so the pre-computed value is returned. After binding, the actual ephemeral port is appended.

This dynamic behavior is intentional for ephemeral port support, but callers may not expect the return value to change over the VirtualHost lifecycle. Consider adding Javadoc clarification, e.g.:

/**
 * Returns the default hostname of this virtual host. If this virtual host is bound to a
 * {`@link` ServerPort} with an ephemeral port (0), the actual bound port is appended after
 * the server starts.
 */

core/src/main/java/com/linecorp/armeria/server/ServerBuilder.java (2)

1860-1903: Add @UnstableApi to the new public overload.
New public APIs should be annotated as unstable per project guidelines.

✅ Proposed fix

+    `@UnstableApi`
     public VirtualHostBuilder virtualHost(ServerPort serverPort) {
         requireNonNull(serverPort, "serverPort");

As per coding guidelines, ...

---

2506-2513: Validate ServerPort by identity, not equals.
contains() uses equals, but runtime matching uses reference identity (==), so a different-but-equal instance would pass validation yet never match.

🐛 Proposed fix

-            if (serverPort != null) {
-                checkState(this.ports.contains(serverPort),
+            if (serverPort != null) {
+                final boolean presentByIdentity = this.ports.stream().anyMatch(p -> p == serverPort);
+                checkState(presentByIdentity,
                            "The ServerPort for a virtual host is not in the server's port list. " +
                            "Please add the ServerPort using port(ServerPort) before creating a virtual host.");
             }

core/src/main/java/com/linecorp/armeria/server/DefaultServerConfig.java (1)

427-445: Consider avoiding per-request full scans when no ServerPort-based vhosts exist.
findVirtualHost(...) now iterates active ports (and then virtual hosts) on every request; a boolean guard or a precomputed map could keep the hot path cheaper.

core/src/main/java/com/linecorp/armeria/server/ServerPort.java (2)

270-277: setActualPort has a non-atomic check-then-act on a volatile field.

The guard checkState(this.actualPort == -1, ...) followed by this.actualPort = actualPort is not atomic. While this is package-private and currently only called from a single event-loop thread in ServerPortStartListener, a future caller from a different thread could trigger a race where two threads both see -1 and both proceed to write.

Consider either:

• Documenting the single-thread assumption (e.g., @GuardedBy or a Javadoc note), or
• Using a CAS via AtomicInteger / VarHandle for correctness.

---

337-345: toString() does not reflect actualPort or originalServerPort.

After binding, actualPort holds meaningful diagnostic information (the real ephemeral port). Including it in toString() would improve debuggability when inspecting bound ports in logs.

core/src/test/java/com/linecorp/armeria/server/PortBasedVirtualHostTest.java (1)

241-293: Consider using ServerPort.actualPort() directly for clarity.

The serverPortBasedVirtualHostRoutesCorrectly and multipleRandomPortVirtualHosts tests access port1.actualPort() directly, which is cleaner than sorting activePorts(). This test could be simplified similarly, though it still works correctly as-is.