Merge pull request #25300 from dmatej/fix-restart-on-fast-machines

Fix creating a lot of ephemeral ports when stopping GlassFish. Fix restart on fast machines

appserver/ejb/ejb-container/src/main/java/org/glassfish/ejb/embedded/EJBContainerImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022 Contributors to the Eclipse Foundation
+ * Copyright (c) 2022, 2024 Contributors to the Eclipse Foundation
  * Copyright (c) 2009, 2020 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -272,7 +272,7 @@ private static class Cleanup implements Runnable {
         Cleanup(EJBContainerImpl container) {
             this.container = container;
             Runtime.getRuntime().addShutdownHook(
-                    cleanupThread = new Thread(this, "EJBContainerImplCleanup"));
+                    cleanupThread = new Thread(this, "GlassFish EJBContainerImpl Cleanup Shutdown Hook"));
         }
 
         void disable() {

appserver/tests/admin/tests/src/test/java/org/glassfish/main/admin/test/rest/RestTestBase.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Contributors to the Eclipse Foundation.
+ * Copyright (c) 2022, 2024 Contributors to the Eclipse Foundation.
  * Copyright (c) 2010, 2018 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -19,19 +19,20 @@
 
 import com.sun.enterprise.util.io.FileUtils;
 
+import jakarta.ws.rs.ProcessingException;
 import jakarta.ws.rs.client.Client;
 import jakarta.ws.rs.core.MultivaluedMap;
 import jakarta.ws.rs.core.Response;
 
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.System.Logger;
+import java.net.ConnectException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 import org.glassfish.admin.rest.client.utils.MarshallingUtils;
 import org.glassfish.main.admin.test.webapp.HelloServlet;
@@ -48,14 +49,16 @@
 
 import static com.sun.enterprise.util.io.FileUtils.ensureWritableDir;
 import static jakarta.ws.rs.core.MediaType.TEXT_PLAIN;
+import static java.lang.System.Logger.Level.ERROR;
+import static java.lang.System.Logger.Level.INFO;
 import static org.glassfish.main.itest.tools.GlassFishTestEnvironment.getTargetDirectory;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 public class RestTestBase {
 
-    private static final Logger LOG = Logger.getLogger(RestTestBase.class.getName());
+    private static final Logger LOG = System.getLogger(RestTestBase.class.getName());
 
     protected static final String CONTEXT_ROOT_MANAGEMENT = "/management";
 
@@ -98,6 +101,12 @@ public static void captureLogAndCloseClient(final TestInfo testInfo) throws Exce
             try (InputStream readEntity = response.readEntity(InputStream.class)) {
                 FileUtils.copy(readEntity, reportFile);
             }
+        } catch (ProcessingException e) {
+            if (e.getCause() instanceof ConnectException) {
+                LOG.log(ERROR, "Unable to backup the server.log using REST, server is not listening on " + baseAdminUrl);
+            } else {
+                throw e;
+            }
         }
     }
 
@@ -160,7 +169,7 @@ public void deleteCluster(String clusterName) {
         Map<String, ?> extraProperties = (Map<String, ?>) body.get("extraProperties");
         if (extraProperties != null) {
             List<Map<String, String>> instanceList = (List<Map<String, String>>) extraProperties.get("instanceList");
-            LOG.log(Level.INFO, "Found instances: {0}", instanceList);
+            LOG.log(INFO, "Found instances: {0}", instanceList);
             if (instanceList != null && !instanceList.isEmpty()) {
                 for (Map<String, String> instance : instanceList) {
                     String instanceName = instance.get("name");
@@ -261,7 +270,7 @@ protected List<String> getCommandResults(Response response) {
 
     protected Map<String, String> getChildResources(Response response) {
         Map responseMap = MarshallingUtils.buildMapFromDocument(response.readEntity(String.class));
-        LOG.log(Level.INFO, "responseMap: \n{0}", responseMap);
+        LOG.log(INFO, "responseMap: \n{0}", responseMap);
         Map<String, Map> extraProperties = (Map<String, Map>) responseMap.get("extraProperties");
         if (extraProperties != null) {
             return extraProperties.get("childResources");
@@ -291,7 +300,7 @@ protected List<Map<String, String>> getProperties(Response response) {
 
     protected static File getEar(final String appName) {
         final EnterpriseArchive ear = ShrinkWrap.create(EnterpriseArchive.class).addAsModule(getWar(appName), "simple.war");
-        LOG.info(ear.toString(true));
+        LOG.log(INFO, ear.toString(true));
         try {
             File tempFile = File.createTempFile(appName, ".ear");
             ear.as(ZipExporter.class).exportTo(tempFile, true);
@@ -303,7 +312,7 @@ protected static File getEar(final String appName) {
 
     protected static File getWar(final String appName) {
         final WebArchive war = ShrinkWrap.create(WebArchive.class).addPackage(HelloServlet.class.getPackage());
-        LOG.info(war.toString(true));
+        LOG.log(INFO, war.toString(true));
         try {
             File tempFile = File.createTempFile(appName, ".war");
             war.as(ZipExporter.class).exportTo(tempFile, true);

appserver/web/web-core/src/main/java/org/apache/catalina/startup/Catalina.java

@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2024 Contributors to the Eclipse Foundation
  * Copyright (c) 1997-2018 Oracle and/or its affiliates. All rights reserved.
  * Copyright 2004 The Apache Software Foundation
  *
@@ -656,6 +657,10 @@ protected void usage() {
      */
     protected class CatalinaShutdownHook extends Thread {
 
+        CatalinaShutdownHook() {
+            super("GlassFish Catalina Shutdown Hook");
+        }
+
         public void run() {
 
             if (server != null) {

nucleus/admin/launcher/src/main/java/com/sun/enterprise/admin/launcher/GFLauncher.java

@@ -870,7 +870,7 @@ private void setShutdownHook(final Process p) {
             final String msg = strings.get("serverStopped", callerParameters.getType());
             processWhacker = new ProcessWhacker(p, msg);
 
-            runtime.addShutdownHook(new Thread(processWhacker));
+            runtime.addShutdownHook(new Thread(processWhacker, "GlassFish Process Whacker Shutdown Hook"));
         } else {
             processWhacker.setProcess(p);
         }

nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/DeleteDomainCommand.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022 Contributors to the Eclipse Foundation
+ * Copyright (c) 2022, 2024 Contributors to the Eclipse Foundation
  * Copyright (c) 1997, 2018 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -81,7 +81,7 @@ protected int executeCommand() throws CommandException, CommandValidationExcepti
 
     private void checkRunning() throws CommandException {
         programOpts.setInteractive(false); // don't prompt for password
-        if (ProcessUtils.isListening(adminAddress) && isThisDAS(getDomainRootDir())) {
+        if (isThisDAS(getDomainRootDir()) && ProcessUtils.isListening(adminAddress)) {
             String msg = strings.get("domain.is.running", getDomainName(), getDomainRootDir());
             throw new IllegalStateException(msg);
         }

nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/LocalServerCommand.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022 Contributors to the Eclipse Foundation
+ * Copyright (c) 2022, 2024 Contributors to the Eclipse Foundation
  * Copyright (c) 1997, 2018 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -43,6 +43,7 @@
 import org.glassfish.api.ActionReport.ExitCode;
 import org.glassfish.api.admin.CommandException;
 
+import static com.sun.enterprise.admin.cli.CLIConstants.DEATH_TIMEOUT_MS;
 import static com.sun.enterprise.admin.cli.CLIConstants.DEFAULT_ADMIN_PORT;
 import static com.sun.enterprise.admin.cli.CLIConstants.DEFAULT_HOSTNAME;
 import static com.sun.enterprise.admin.cli.ProgramOptions.PasswordLocation.LOCAL_PASSWORD;
@@ -331,22 +332,15 @@ protected final void waitForRestart(final int oldPid, final HostAndPort oldAdmin
         logger.log(Level.FINEST, "waitForRestart(oldPid={0}, oldAdminAddress={1}, newAdminAddress={2})",
             new Object[] {oldPid, oldAdminAddress, newAdminAddress});
 
-        final Supplier<Boolean> signStop = () -> {
-            if (!ProcessUtils.isListening(oldAdminAddress)) {
-                return true;
-            }
-            int newPid = getServerPid();
-            if (newPid < 0) {
-                // is listening, but not responding.
-                // Could be also another application, but then
-                // remote _restart-domain call should already fail.
-                return true;
-            }
-            // stopped and started again
-            return oldPid != newPid;
-        };
+        final Duration timeout = Duration.ofMillis(DEATH_TIMEOUT_MS);
         final boolean printDots = !programOpts.isTerse();
-        if (!ProcessUtils.waitFor(signStop, Duration.ofMillis(CLIConstants.DEATH_TIMEOUT_MS), printDots)) {
+        final boolean stopped;
+        if (isLocal()) {
+            stopped = ProcessUtils.waitWhileIsAlive(oldPid, timeout, printDots);
+        } else {
+            stopped = ProcessUtils.waitWhileListening(oldAdminAddress, timeout, printDots);
+        }
+        if (!stopped) {
             throw new CommandException(I18N.get("restartDomain.noGFStart"));
         }
         logger.log(CONFIG, "Server instance is stopped, now we wait for the start on {0}", newAdminAddress);
@@ -383,7 +377,7 @@ protected final void waitForRestart(final int oldPid, final HostAndPort oldAdmin
 
 
     /**
-     * Get uptime from the server.
+     * @return uptime from the server.
      */
     protected final long getUptime() throws CommandException {
         RemoteCLICommand cmd = new RemoteCLICommand("uptime", programOpts, env);

nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/StartServerHelper.java

@@ -78,21 +78,21 @@ public StartServerHelper(boolean terse, ServerDirs serverDirs, GFLauncher launch
             boolean debug) {
         this.terse = terse;
         this.launcher = launcher;
-        info = launcher.getInfo();
+        this.info = launcher.getInfo();
 
         if (info.isDomain()) {
-            serverOrDomainName = info.getDomainName();
+            this.serverOrDomainName = info.getDomainName();
         } else {
-            serverOrDomainName = info.getInstanceName();
+            this.serverOrDomainName = info.getInstanceName();
         }
 
-        addresses = info.getAdminAddresses();
+        this.addresses = info.getAdminAddresses();
         this.serverDirs = serverDirs;
-        pidFile = serverDirs.getPidFile();
+        this.pidFile = serverDirs.getPidFile();
         this.masterPassword = masterPassword;
 
         // it will be < 0 if both --debug is false and debug-enabled=false in jvm-config
-        debugPort = launcher.getDebugPort();
+        this.debugPort = launcher.getDebugPort();
     }
 
 
@@ -205,13 +205,7 @@ private void waitForParentToDie() throws CommandException {
             return;
         }
         LOG.log(DEBUG, "Waiting for death of the parent process with pid={0}", pid);
-        final Supplier<Boolean> parentDeathSign = () -> {
-            if (pid != null && ProcessUtils.isAlive(pid)) {
-                return false;
-            }
-            return !isListeningOnAnyEndpoint();
-        };
-        if (!ProcessUtils.waitFor(parentDeathSign, Duration.ofMillis(DEATH_TIMEOUT_MS), false)) {
+        if (!ProcessUtils.waitWhileIsAlive(pid, Duration.ofMillis(DEATH_TIMEOUT_MS), false)) {
             throw new CommandException(I18N.get("deathwait_timeout", DEATH_TIMEOUT_MS));
         }
         LOG.log(DEBUG, "Parent process with PID={0} is dead and all admin endpoints are free.", pid);

nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/StopDomainCommand.java

@@ -28,7 +28,6 @@
 import java.lang.System.Logger;
 import java.lang.System.Logger.Level;
 import java.time.Duration;
-import java.util.function.Supplier;
 
 import org.glassfish.api.Param;
 import org.glassfish.api.admin.CommandException;
@@ -143,8 +142,7 @@ protected int dasNotRunning() throws CommandException {
             if (isLocal()) {
                 try {
                     File prevPid = getServerDirs().getLastPidFile();
-                    File watchedPid = getServerDirs().getPidFile();
-                    ProcessUtils.kill(prevPid, watchedPid, Duration.ofMillis(DEATH_TIMEOUT_MS), !programOpts.isTerse());
+                    ProcessUtils.kill(prevPid, Duration.ofMillis(DEATH_TIMEOUT_MS), !programOpts.isTerse());
                 } catch (KillNotPossibleException e) {
                     throw new CommandException(e.getMessage(), e);
                 }
@@ -169,19 +167,34 @@ protected int dasNotRunning() throws CommandException {
      */
     protected void doCommand() throws CommandException {
         // run the remote stop-domain command and throw away the output
-        RemoteCLICommand cmd = new RemoteCLICommand(getName(), programOpts, env);
-        File watchedPid = isLocal() ? getServerDirs().getPidFile() : null;
+        final RemoteCLICommand cmd = new RemoteCLICommand(getName(), programOpts, env);
+        final File watchedPid = isLocal() ? getServerDirs().getPidFile() : null;
+        final Long oldPid = ProcessUtils.loadPid(watchedPid);
+        final Duration timeout = Duration.ofMillis(DEATH_TIMEOUT_MS);
+        final boolean printDots = !programOpts.isTerse();
         try {
             cmd.executeAndReturnOutput("stop-domain", "--force", force.toString());
-            waitForDeath(watchedPid);
+            if (printDots) {
+                // use stdout because logger always appends a newline
+                System.out.print(Strings.get("StopDomain.WaitDASDeath") + " ");
+            }
+            final boolean dead;
+            if (isLocal()) {
+                dead = ProcessUtils.waitWhileIsAlive(oldPid, timeout, printDots);
+            } else {
+                dead = ProcessUtils.waitWhileListening(addr, timeout, printDots);
+            }
+            if (!dead) {
+                throw new CommandException(Strings.get("StopDomain.DASNotDead", timeout.toSeconds()));
+            }
         } catch (Exception e) {
             // The domain server may have died so fast we didn't have time to
             // get the (always successful!!) return data.  This is NOT AN ERROR!
             LOG.log(Level.DEBUG, "Remote stop-domain call failed.", e);
             if (kill && isLocal()) {
                 try {
                     File prevPid = getServerDirs().getLastPidFile();
-                    ProcessUtils.kill(prevPid, watchedPid, Duration.ofMillis(DEATH_TIMEOUT_MS), !programOpts.isTerse());
+                    ProcessUtils.kill(prevPid, timeout, printDots);
                     return;
                 } catch (Exception ex) {
                     e.addSuppressed(ex);
@@ -190,27 +203,4 @@ protected void doCommand() throws CommandException {
             throw e;
         }
     }
-
-    /**
-     * Wait for the server to die.
-     *
-     * @param watchedPid
-     */
-    private void waitForDeath(File watchedPid) throws CommandException {
-        if (!programOpts.isTerse()) {
-            // use stdout because logger always appends a newline
-            System.out.print(Strings.get("StopDomain.WaitDASDeath") + " ");
-        }
-        final Duration timeout = Duration.ofMillis(DEATH_TIMEOUT_MS);
-        final Supplier<Boolean> deathSign;
-        if (isLocal()) {
-            deathSign = () -> !ProcessUtils.isListening(addr) && !ProcessUtils.isAlive(watchedPid);
-        } else {
-            deathSign = () -> !ProcessUtils.isListening(addr);
-        }
-        final boolean dead = ProcessUtils.waitFor(deathSign, timeout, !programOpts.isTerse());
-        if (!dead) {
-            throw new CommandException(Strings.get("StopDomain.DASNotDead", timeout.toSeconds()));
-        }
-    }
 }