diff --git a/src/main/java/org/wildfly/plugin/tools/ServerHelper.java b/src/main/java/org/wildfly/plugin/tools/ServerHelper.java index 3751648..7ce2caa 100644 --- a/src/main/java/org/wildfly/plugin/tools/ServerHelper.java +++ b/src/main/java/org/wildfly/plugin/tools/ServerHelper.java @@ -30,7 +30,7 @@ * @see StandaloneManager * @see DomainManager */ -@SuppressWarnings("unused") +@SuppressWarnings({ "unused", "resource" }) @Deprecated(forRemoval = true, since = "1.1") public class ServerHelper { @@ -98,7 +98,11 @@ public static void reloadIfRequired(final ModelControllerClient client, final lo * @param reloadOp the reload operation to execute */ public static void executeReload(final ModelControllerClient client, final ModelNode reloadOp) { - ServerManager.builder().client(client).standalone().executeReload(reloadOp); + try { + ServerManager.builder().client(client).standalone().executeReload(reloadOp); + } catch (IOException e) { + throw new UncheckedIOException(e); + } } /** diff --git a/src/main/java/org/wildfly/plugin/tools/server/AbstractServerManager.java b/src/main/java/org/wildfly/plugin/tools/server/AbstractServerManager.java index d27ffd8..d4681d0 100644 --- a/src/main/java/org/wildfly/plugin/tools/server/AbstractServerManager.java +++ b/src/main/java/org/wildfly/plugin/tools/server/AbstractServerManager.java @@ -10,6 +10,7 @@ import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import org.jboss.as.controller.client.ModelControllerClient; import org.jboss.as.controller.client.helpers.Operations; @@ -25,15 +26,29 @@ * @author James R. Perkins */ @SuppressWarnings("unused") -abstract class AbstractServerManager implements ServerManager { +abstract class AbstractServerManager implements ServerManager { private static final Logger LOGGER = Logger.getLogger(AbstractServerManager.class); + private static final ThreadLocal SKIP_STATE_CHECK = ThreadLocal.withInitial(() -> false); protected final ProcessHandle process; + final T client; + private final boolean shutdownOnClose; private final DeploymentManager deploymentManager; + private final AtomicBoolean closed; - protected AbstractServerManager(final ProcessHandle process, final ModelControllerClient client) { + protected AbstractServerManager(final ProcessHandle process, final T client, + final boolean shutdownOnClose) { this.process = process; + this.client = client; + this.shutdownOnClose = shutdownOnClose; deploymentManager = DeploymentManager.Factory.create(client); + this.closed = new AtomicBoolean(false); + } + + @Override + public ModelControllerClient client() { + checkState(); + return client; } @Override @@ -109,14 +124,48 @@ public String takeSnapshot() throws IOException, OperationExecutionException { * @throws OperationExecutionException if the reload operation fails */ @Override - public void executeReload(final ModelNode reloadOp) throws OperationExecutionException { + public void executeReload(final ModelNode reloadOp) throws IOException, OperationExecutionException { try { executeOperation(reloadOp); } catch (IOException e) { final Throwable cause = e.getCause(); if (!(cause instanceof ExecutionException) && !(cause instanceof CancellationException)) { - throw new RuntimeException(e); + throw e; } // else ignore, this might happen if the channel gets closed before we got the response } } + + @Override + public boolean isClosed() { + return closed.get(); + } + + @Override + public void close() { + if (closed.compareAndSet(false, true)) { + try { + SKIP_STATE_CHECK.set(true); + if (shutdownOnClose) { + try { + shutdown(); + } catch (IOException e) { + LOGGER.error("Failed to shutdown the server while closing the server manager.", e); + } + } + try { + client.close(); + } catch (IOException e) { + LOGGER.error("Failed to close the client.", e); + } + } finally { + SKIP_STATE_CHECK.remove(); + } + } + } + + void checkState() { + if (!SKIP_STATE_CHECK.get() && closed.get()) { + throw new IllegalStateException("The server manager has been closed and cannot process requests"); + } + } } diff --git a/src/main/java/org/wildfly/plugin/tools/server/DomainManager.java b/src/main/java/org/wildfly/plugin/tools/server/DomainManager.java index a1d89d6..ac6f431 100644 --- a/src/main/java/org/wildfly/plugin/tools/server/DomainManager.java +++ b/src/main/java/org/wildfly/plugin/tools/server/DomainManager.java @@ -11,7 +11,6 @@ import java.util.Map; import java.util.concurrent.TimeUnit; -import org.jboss.as.controller.client.ModelControllerClient; import org.jboss.as.controller.client.helpers.ClientConstants; import org.jboss.as.controller.client.helpers.Operations; import org.jboss.as.controller.client.helpers.domain.DomainClient; @@ -26,18 +25,12 @@ * @author James R. Perkins */ @SuppressWarnings("unused") -public class DomainManager extends AbstractServerManager { +public class DomainManager extends AbstractServerManager { private static final Logger LOGGER = Logger.getLogger(DomainManager.class); - private final DomainClient client; - protected DomainManager(final ProcessHandle process, final DomainClient client) { - super(process, client); - this.client = client; - } - - @Override - public ModelControllerClient client() { - return client; + DomainManager(final ProcessHandle process, final DomainClient client, + final boolean shutdownOnClose) { + super(process, client, shutdownOnClose); } @Override @@ -62,7 +55,7 @@ public String serverState() { * @throws OperationExecutionException if the operation used to determine the host name fails */ public ModelNode determineHostAddress() throws OperationExecutionException, IOException { - return CommonOperations.determineHostAddress(client); + return CommonOperations.determineHostAddress(client()); } /** @@ -73,7 +66,10 @@ public ModelNode determineHostAddress() throws OperationExecutionException, IOEx */ @Override public boolean isRunning() { - return CommonOperations.isDomainRunning(client, false); + if (process != null) { + return process.isAlive() && CommonOperations.isDomainRunning(client(), false); + } + return CommonOperations.isDomainRunning(client(), false); } @Override @@ -101,7 +97,8 @@ public void shutdown(final long timeout) executeOperation(shutdownOp); // Wait until the process has died while (true) { - if (CommonOperations.isDomainRunning(client, true)) { + final boolean running = (process == null ? CommonOperations.isDomainRunning(client(), true) : process.isAlive()); + if (running) { Thread.onSpinWait(); } else { break; @@ -110,7 +107,7 @@ public void shutdown(final long timeout) } @Override - public void executeReload() { + public void executeReload() throws IOException, OperationExecutionException { executeReload(Operations.createOperation("reload-servers")); } diff --git a/src/main/java/org/wildfly/plugin/tools/server/ServerManager.java b/src/main/java/org/wildfly/plugin/tools/server/ServerManager.java index 9a19c2b..58f4ea0 100644 --- a/src/main/java/org/wildfly/plugin/tools/server/ServerManager.java +++ b/src/main/java/org/wildfly/plugin/tools/server/ServerManager.java @@ -25,11 +25,14 @@ /** * A simple manager for various interactions with a potentially running server. + *

+ * When the server manager is closed, the underlying client is also closed. + *

* * @author James R. Perkins */ @SuppressWarnings("unused") -public interface ServerManager { +public interface ServerManager extends AutoCloseable { /** * A builder used to build a {@link ServerManager}. @@ -39,9 +42,10 @@ class Builder { private String managementAddress; private int managementPort; private ProcessHandle process; + private boolean shutdownOnClose; /** - * Sets the client to use for the server manager. + * Sets the client to use for the server manager. The caller is responsible for closing the client. * * @param client the client to use to communicate with the server * @@ -105,6 +109,19 @@ public Builder managementPort(final int managementPort) { return this; } + /** + * When set to {@code true} the server will be {@linkplain ServerManager#shutdown() shutdown} when the server + * manager is {@linkplain ServerManager#close() closed}. + * + * @param shutdownOnClose {@code true} to shutdown the server when the server manager is closed + * + * @return this builder + */ + public Builder shutdownOnClose(final boolean shutdownOnClose) { + this.shutdownOnClose = shutdownOnClose; + return this; + } + /** * Creates a {@link StandaloneManager} based on the builders settings. If the * {@link #client(ModelControllerClient) client} was not set, the {@link #managementAddress(String) managementAddress} @@ -113,7 +130,7 @@ public Builder managementPort(final int managementPort) { * @return a new {@link StandaloneManager} */ public StandaloneManager standalone() { - return new StandaloneManager(process, getOrCreateClient()); + return new StandaloneManager(process, getOrCreateClient(), shutdownOnClose); } /** @@ -124,7 +141,7 @@ public StandaloneManager standalone() { * @return a new {@link DomainManager} */ public DomainManager domain() { - return new DomainManager(process, getOrCreateDomainClient()); + return new DomainManager(process, getOrCreateDomainClient(), shutdownOnClose); } /** @@ -153,9 +170,9 @@ public CompletableFuture build() { final String launchType = launchType(client).orElseThrow(() -> new IllegalStateException( "Could not determine the type of the server. Verify the server is running.")); if ("STANDALONE".equals(launchType)) { - return new StandaloneManager(process, client); + return new StandaloneManager(process, client, shutdownOnClose); } else if ("DOMAIN".equals(launchType)) { - return new DomainManager(process, getOrCreateDomainClient()); + return new DomainManager(process, getOrCreateDomainClient(), shutdownOnClose); } throw new IllegalStateException( String.format("Only standalone and domain servers are support. %s is not supported.", launchType)); @@ -475,4 +492,25 @@ default ModelNode executeOperation(final Operation op) throws IOException, Opera } throw new OperationExecutionException(op, result); } + + /** + * Checks if this server manager has been closed. The server manager may be closed if the underlying client was + * closed. + * + * @return {@code true} if the server manager was closed, otherwise {@code false} + * @since 1.2 + */ + default boolean isClosed() { + throw new UnsupportedOperationException("Not yet implemented"); + } + + /** + * {@inheritDoc} + * + * @since 1.2 + */ + @Override + default void close() { + throw new UnsupportedOperationException("Not yet implemented"); + } } diff --git a/src/main/java/org/wildfly/plugin/tools/server/StandaloneManager.java b/src/main/java/org/wildfly/plugin/tools/server/StandaloneManager.java index 8508df4..05453b3 100644 --- a/src/main/java/org/wildfly/plugin/tools/server/StandaloneManager.java +++ b/src/main/java/org/wildfly/plugin/tools/server/StandaloneManager.java @@ -21,18 +21,12 @@ * @author James R. Perkins */ @SuppressWarnings("unused") -public class StandaloneManager extends AbstractServerManager { +public class StandaloneManager extends AbstractServerManager { private static final Logger LOGGER = Logger.getLogger(StandaloneManager.class); - private final ModelControllerClient client; - protected StandaloneManager(final ProcessHandle process, final ModelControllerClient client) { - super(process, client); - this.client = client; - } - - @Override - public ModelControllerClient client() { - return client; + StandaloneManager(final ProcessHandle process, final ModelControllerClient client, + final boolean shutdownOnClose) { + super(process, client, shutdownOnClose); } @Override @@ -81,7 +75,10 @@ public void reloadIfRequired(final long timeout, final TimeUnit unit) throws IOE @Override public boolean isRunning() { - return CommonOperations.isStandaloneRunning(client); + if (process != null) { + return process.isAlive() && CommonOperations.isStandaloneRunning(client()); + } + return CommonOperations.isStandaloneRunning(client()); } @Override @@ -95,7 +92,8 @@ public void shutdown(final long timeout) throws IOException { op.get("timeout").set(timeout); executeOperation(op); while (true) { - if (isRunning()) { + final boolean running = (process == null ? isRunning() : process.isAlive()); + if (running) { Thread.onSpinWait(); } else { break; diff --git a/src/test/java/org/wildfly/plugin/tools/Environment.java b/src/test/java/org/wildfly/plugin/tools/Environment.java index ece8d88..bcb0543 100644 --- a/src/test/java/org/wildfly/plugin/tools/Environment.java +++ b/src/test/java/org/wildfly/plugin/tools/Environment.java @@ -81,6 +81,20 @@ public static ModelControllerClient createClient() throws UnknownHostException { return ModelControllerClient.Factory.create(HOSTNAME, PORT); } + /** + * Creates a {@link ServerManager.Builder} for testing with a default management address and port. + * + * @param process the process to bind to the server manager + * + * @return a server manager builder + */ + public static ServerManager.Builder createServerManagerBuilder(final Process process) { + return ServerManager.builder() + .managementAddress(HOSTNAME) + .managementPort(PORT) + .process(process); + } + private static void validateWildFlyHome(final Path wildflyHome) { if (!ServerManager.isValidHomeDirectory(wildflyHome)) { throw new RuntimeException("Invalid WildFly home directory: " + wildflyHome); diff --git a/src/test/java/org/wildfly/plugin/tools/ServerManagerIT.java b/src/test/java/org/wildfly/plugin/tools/ServerManagerIT.java index 236c6bd..17ded3d 100644 --- a/src/test/java/org/wildfly/plugin/tools/ServerManagerIT.java +++ b/src/test/java/org/wildfly/plugin/tools/ServerManagerIT.java @@ -56,6 +56,7 @@ public void checkStandaloneServerState() throws Exception { public void checkStandaloneReloadIfRequired() throws Exception { final Process process = launchStandalone(); try (ModelControllerClient client = Environment.createClient()) { + @SuppressWarnings("resource") final StandaloneManager serverManager = ServerManager.builder() .process(process) .client(client) @@ -82,6 +83,7 @@ public void checkStandaloneReloadIfRequired() throws Exception { public void checkDomainReloadIfRequired() throws Exception { final Process process = launchDomain(); try (ModelControllerClient client = Environment.createClient()) { + @SuppressWarnings("resource") final DomainManager serverManager = ServerManager.builder().process(process).client(client).domain(); serverManager.waitFor(Environment.TIMEOUT, TimeUnit.SECONDS); // Execute a command which will put the server in a state of requiring a reload @@ -103,6 +105,73 @@ public void checkDomainReloadIfRequired() throws Exception { } } + @Test + public void checkStandaloneServerManagerClosed() throws Exception { + final Process process = launchStandalone(); + ServerManager checker; + try ( + ServerManager serverManager = ServerManager.builder().process(process) + .managementAddress(Environment.HOSTNAME).managementPort(Environment.PORT).standalone()) { + serverManager.waitFor(Environment.TIMEOUT, TimeUnit.SECONDS); + checker = serverManager; + serverManager.shutdown(); + } finally { + process.destroyForcibly(); + } + Assertions.assertTrue(checker.isClosed(), String + .format("Expected ServerManager %s to be closed, but the client did not close the server manager.", checker)); + } + + @Test + public void checkStandaloneShutdownOnClose() throws Exception { + final Process process = launchStandalone(); + try { + try ( + ServerManager serverManager = ServerManager.builder().process(process) + .shutdownOnClose(true) + .managementAddress(Environment.HOSTNAME).managementPort(Environment.PORT).standalone()) { + serverManager.waitFor(Environment.TIMEOUT, TimeUnit.SECONDS); + Assertions.assertTrue(serverManager.isRunning(), "The server does not appear to be running."); + } + Assertions.assertFalse(process.isAlive(), "Expected server to be shutdown"); + } finally { + process.destroyForcibly(); + } + } + + @Test + public void checkDomainServerManagerClosed() throws Exception { + final Process process = launchDomain(); + ServerManager checker; + try ( + ServerManager serverManager = ServerManager.builder().process(process) + .managementAddress(Environment.HOSTNAME).managementPort(Environment.PORT).domain()) { + serverManager.waitFor(Environment.TIMEOUT, TimeUnit.SECONDS); + checker = serverManager; + serverManager.shutdown(); + } finally { + process.destroyForcibly(); + } + Assertions.assertTrue(checker.isClosed(), String + .format("Expected ServerManager %s to be closed, but the client did not close the server manager.", checker)); + } + + @Test + public void checkDomainShutdownOnClose() throws Exception { + final Process process = launchDomain(); + try { + try ( + ServerManager serverManager = ServerManager.builder().process(process) + .shutdownOnClose(true) + .managementAddress(Environment.HOSTNAME).managementPort(Environment.PORT).domain()) { + serverManager.waitFor(Environment.TIMEOUT, TimeUnit.SECONDS); + } + Assertions.assertFalse(process.isAlive(), "Expected server to be shutdown"); + } finally { + process.destroyForcibly(); + } + } + private ModelNode executeCommand(final ModelControllerClient client, final ModelNode op) throws IOException { final ModelNode result = client.execute(op); if (!Operations.isSuccessfulOutcome(result)) { @@ -131,33 +200,25 @@ private void verifyReloadRequired(final ModelNode result) { } private void checkServerState(final Process process) throws Exception { - try (ModelControllerClient client = Environment.createClient()) { - final ServerManager serverManager = ServerManager.builder().process(process).client(client).build() - .get(Environment.TIMEOUT, TimeUnit.SECONDS); - try { - Assertions.assertTrue(serverManager.waitFor(Environment.TIMEOUT), "Server failed to start"); - // Check the server state - Assertions.assertEquals("running", serverManager.serverState()); - } finally { - serverManager.shutdown(); - } + try ( + ServerManager serverManager = Environment.createServerManagerBuilder(process).build() + .get(Environment.TIMEOUT, TimeUnit.SECONDS)) { + Assertions.assertTrue(serverManager.waitFor(Environment.TIMEOUT), "Server failed to start"); + // Check the server state + Assertions.assertEquals("running", serverManager.serverState()); } finally { process.destroyForcibly(); } } private void checkProcess(final Process process) throws Exception { - try (ModelControllerClient client = Environment.createClient()) { - final ServerManager serverManager = ServerManager.builder().process(process).client(client).build() - .get(Environment.TIMEOUT, TimeUnit.SECONDS); - try { - Assertions.assertTrue(serverManager.waitFor(Environment.TIMEOUT), "Server failed to start"); - final Optional handle = ServerManager.findProcess(); - Assertions.assertTrue(handle.isPresent(), () -> "Could not find the server process for " + process.pid()); - Assertions.assertEquals(process.toHandle(), handle.get()); - } finally { - serverManager.shutdown(); - } + try ( + ServerManager serverManager = Environment.createServerManagerBuilder(process).build() + .get(Environment.TIMEOUT, TimeUnit.SECONDS)) { + Assertions.assertTrue(serverManager.waitFor(Environment.TIMEOUT), "Server failed to start"); + final Optional handle = ServerManager.findProcess(); + Assertions.assertTrue(handle.isPresent(), () -> "Could not find the server process for " + process.pid()); + Assertions.assertEquals(process.toHandle(), handle.get()); } finally { process.destroyForcibly(); }