Merge pull request #25790 from dmatej/restarts

More flexibility to control server start

Author: arjantijms

appserver/itest-tools/src/main/java/org/glassfish/main/itest/tools/asadmin/Asadmin.java

@@ -242,7 +242,7 @@ private File getPasswordFile() {
      */
     private AsadminResult exec(final Integer timeout, final boolean detachedAndTerse, final String... args) {
         final List<String> parameters = Arrays.asList(args);
-        LOG.log(TRACE, "exec(timeout={0}, detached={1}, args={2})", timeout, detachedAndTerse, parameters);
+        LOG.log(INFO, "exec(timeout={0}, detached={1}, args={2})", timeout, detachedAndTerse, parameters);
         final List<String> command = new ArrayList<>();
         if (asadmin.getName().endsWith(".java")) {
             command.add(JAVA_EXECUTABLE);

appserver/orb/orb-connector/src/main/java/org/glassfish/enterprise/iiop/api/GlassFishORBHelper.java

@@ -17,29 +17,28 @@
 
 package org.glassfish.enterprise.iiop.api;
 
-import com.sun.logging.LogDomains;
-
 import jakarta.annotation.PostConstruct;
-import jakarta.annotation.PreDestroy;
 import jakarta.ejb.Singleton;
 import jakarta.inject.Inject;
 import jakarta.inject.Provider;
 
+import java.lang.System.Logger;
 import java.nio.channels.SelectableChannel;
 import java.rmi.Remote;
 import java.util.Properties;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 import org.glassfish.api.admin.ProcessEnvironment;
+import org.glassfish.api.event.EventListener;
+import org.glassfish.api.event.Events;
 import org.glassfish.api.naming.GlassfishNamingManager;
 import org.glassfish.hk2.api.ServiceLocator;
 import org.glassfish.internal.api.ORBLocator;
 import org.jvnet.hk2.annotations.Service;
 import org.omg.CORBA.ORB;
 import org.omg.PortableInterceptor.ServerRequestInfo;
 
-import static com.sun.logging.LogDomains.CORBA_LOGGER;
+import static java.lang.System.Logger.Level.INFO;
+import static org.glassfish.api.event.EventTypes.SERVER_SHUTDOWN;
 
 /**
  * This class exposes any orb/iiop functionality needed by modules in the app server.
@@ -51,7 +50,10 @@
 @Singleton
 public class GlassFishORBHelper implements ORBLocator {
 
-    private static final Logger LOG = LogDomains.getLogger(GlassFishORBHelper.class, CORBA_LOGGER, false);
+    private static final Logger LOG = System.getLogger(GlassFishORBHelper.class.getName());
+
+    @Inject
+    private Provider<Events> eventsProvider;
 
     @Inject
     private ServiceLocator services;
@@ -75,17 +77,35 @@ public class GlassFishORBHelper implements ORBLocator {
 
     @PostConstruct
     public void postConstruct() {
+        // WARN: Neither PreDestroy annotation nor interface worked!
+        EventListener glassfishEventListener = event -> {
+            if (event.is(SERVER_SHUTDOWN)) {
+                onShutdown();
+            }
+        };
+        eventsProvider.get().register(glassfishEventListener);
         orbFactory = services.getService(GlassFishORBFactory.class);
+        LOG.log(INFO, "GlassFishORBLocator created.");
     }
 
-    @PreDestroy
-    public void onShutdown() {
+    private void onShutdown() {
         // FIXME: getORB is able to create another, it should be refactored and simplified.
         destroyed = true;
-        LOG.log(Level.CONFIG, "ORB Shutdown started");
-        if (orb != null) {
-            orb.destroy();
+        LOG.log(INFO, "ORB shutdown started");
+        if (this.orb != null) {
+            // First remove, then destroy.
+            // Still, threads already working with the instance will have it unstable.
+            final ORB destroyedOrb = orb;
             orb = null;
+            // FIXME: com.sun.corba.ee.impl.transport.AcceptorImpl.getAcceptedSocket(AcceptorImpl.java:127)
+            //        can still be blocked in standalone thread, that would lead to its failure
+            //        and cascade leading sockets open. Restart of the server could fail then.
+            try {
+                Thread.sleep(1000L);
+            } catch (InterruptedException e) {
+                // We don't want to interrupt here.
+            }
+            destroyedOrb.destroy();
         }
     }
 

appserver/orb/orb-iiop/src/main/java/org/glassfish/enterprise/iiop/impl/IIOPSSLSocketFactory.java

@@ -32,7 +32,10 @@
 import java.net.SocketException;
 import java.nio.channels.ServerSocketChannel;
 import java.nio.channels.SocketChannel;
+import java.time.Duration;
+import java.time.Instant;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
@@ -89,7 +92,7 @@ public class IIOPSSLSocketFactory implements ORBSocketFactory {
      * @todo provide an interface to the admin, so that whenever a iiop-listener
      * is added / removed, we modify the hashtable,
      */
-    private final Map portToSSLInfo = new Hashtable();
+    private final Map<Integer, SSLInfo> portToSSLInfo = new Hashtable<>();
     /* this is stored for the client side of SSL Connections.
      * Note: There will be only 1 ctx for the client side, as we will reuse the
      * ctx for all SSL connections
@@ -175,8 +178,7 @@ public IIOPSSLSocketFactory() {
                 }
             }
         } catch (Exception e) {
-            LOG.log(Level.SEVERE,"IIOPSSLSocketFactory initialization failed.", e);
-            throw new IllegalStateException(e);
+            throw new IllegalStateException("IIOPSSLSocketFactory initialization failed.", e);
         }
     }
 
@@ -251,25 +253,23 @@ public void setORB(ORB orb) {
      */
     @Override
     public ServerSocket createServerSocket(String type, InetSocketAddress inetSocketAddress) throws IOException {
-        if (LOG.isLoggable(Level.FINE)) {
-            LOG.log(Level.FINE, "Creating server socket for type =" + type
-                    + " inetSocketAddress =" + inetSocketAddress);
-        }
+        LOG.log(Level.INFO, "Creating server socket for type =" + type + " inetSocketAddress =" + inetSocketAddress);
 
-        if(type.equals(SSL_MUTUALAUTH) || type.equals(SSL) ||
-            type.equals(PERSISTENT_SSL)) {
+        if (type.equals(SSL_MUTUALAUTH) || type.equals(SSL) || type.equals(PERSISTENT_SSL)) {
             return createSSLServerSocket(type, inetSocketAddress);
         }
-        ServerSocket serverSocket = null;
+        final ServerSocket serverSocket;
         if (orb.getORBData().acceptorSocketType().equals(SOCKETCHANNEL)) {
             ServerSocketChannel serverSocketChannel = ServerSocketChannel.open();
             serverSocket = serverSocketChannel.socket();
         } else {
             serverSocket = new ServerSocket();
         }
-
-         serverSocket.bind(inetSocketAddress);
-         return serverSocket;
+        final Action<ServerSocket> action = () -> {
+            serverSocket.bind(inetSocketAddress);
+            return serverSocket;
+        };
+        return repeat(action, Duration.ofSeconds(10L));
     }
 
     /**
@@ -339,7 +339,7 @@ private ServerSocket createSSLServerSocket(String type, InetSocketAddress inetSo
         }
         int port = inetSocketAddress.getPort();
         Integer iport = Integer.valueOf(port);
-        SSLInfo sslInfo = (SSLInfo)portToSSLInfo.get(iport);
+        SSLInfo sslInfo = portToSSLInfo.get(iport);
         if (sslInfo == null) {
             throw new IOException("No SSL info found for port " + iport);
         }
@@ -351,42 +351,21 @@ private ServerSocket createSSLServerSocket(String type, InetSocketAddress inetSo
             String[] socketCiphers = ssf.getDefaultCipherSuites();
             ciphers = mergeCiphers(socketCiphers, ssl3TlsCiphers, ssl2Ciphers);
         }
-
-        String cs[] = null;
-
-        if (LOG.isLoggable(Level.FINE)) {
-            cs = ssf.getSupportedCipherSuites();
-            for (String element : cs) {
-                LOG.log(Level.FINE, "Cipher Suite: " + element);
-            }
+        LOG.log(Level.FINE, () -> "Supported cipher Suites: " + Arrays.toString(ssf.getSupportedCipherSuites()));
+        final Action<ServerSocket> action = () -> ssf.createServerSocket(port, BACKLOG, inetSocketAddress.getAddress());
+        final ServerSocket ss = repeat(action, Duration.ofSeconds(10L));
+        if (ciphers != null) {
+            ((SSLServerSocket) ss).setEnabledCipherSuites(ciphers);
         }
-        ServerSocket ss = null;
-        try {
-            // bugfix for 6349541
-            // specify the ip address to bind to, 50 is the default used
-            // by the ssf implementation when only the port is specified
-            ss = ssf.createServerSocket(port, BACKLOG, inetSocketAddress.getAddress());
-            if (ciphers != null) {
-                ((SSLServerSocket) ss).setEnabledCipherSuites(ciphers);
-            }
-        } catch (IOException e) {
-            LOG.log(Level.SEVERE, "createServerSocket failed", new Object[] {type, port});
-            LOG.log(Level.SEVERE, "", e);
-            throw e;
-        }
-
         try {
             if (type.equals(SSL_MUTUALAUTH)) {
                 LOG.log(Level.FINE, "Setting Mutual auth");
                 ((SSLServerSocket) ss).setNeedClientAuth(true);
             }
         } catch (Exception e) {
-            LOG.log(Level.SEVERE, "Setting Mutual auth failed.", e);
-            throw new IOException(e.getMessage());
-        }
-        if (LOG.isLoggable(Level.FINE)) {
-            LOG.log(Level.FINE, "Created server socket:" + ss);
+            throw new IOException(e.getMessage(), e);
         }
+        LOG.log(Level.FINE, () -> "Created server socket: " + ss);
         return ss;
     }
 
@@ -424,9 +403,7 @@ private Socket createSSLSocket(String host, int port) throws IOException {
                 LOG.log(Level.FINE, "createSSLSocket failed.", new Object[] {host, port});
                 LOG.log(Level.FINE, "", e);
             }
-            IOException e2 = new IOException("Error opening SSL socket to host=" + host + " port=" + port);
-            e2.initCause(e);
-            throw e2;
+            throw new IOException("Error opening SSL socket to host=" + host + " port=" + port, e);
         }
         return socket;
     }
@@ -550,6 +527,20 @@ private boolean isValidProtocolCipher(CipherInfo cipherInfo,
     }
 
 
+    private static <T> T repeat(Action<T> action, Duration timeout) throws IOException {
+        final Instant deadline = Instant.now().plus(timeout);
+        while (true) {
+            try {
+                return action.get();
+            } catch (Exception e) {
+                if (Instant.now().isAfter(deadline)) {
+                    throw e;
+                }
+            }
+        }
+    }
+
+
     class SSLInfo {
         private final SSLContext ctx;
         private String[] ssl3TlsCiphers = null;
@@ -573,4 +564,9 @@ String[] getSsl2Ciphers() {
             return ssl2Ciphers;
         }
     }
+
+    @FunctionalInterface
+    private interface Action<T> {
+        T get() throws IOException;
+    }
 }

appserver/tests/admin/tests/src/test/java/org/glassfish/main/admin/test/AsadminLoggingITest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2024 Contributors to the Eclipse Foundation.
+ * Copyright (c) 2022, 2025 Contributors to the Eclipse Foundation.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0, which is available at
@@ -16,6 +16,8 @@
 
 package org.glassfish.main.admin.test;
 
+import com.sun.enterprise.util.OS;
+
 import java.io.File;
 import java.io.FileReader;
 import java.io.LineNumberReader;
@@ -69,11 +71,16 @@ public class AsadminLoggingITest {
 
     private static final Asadmin ASADMIN = GlassFishTestEnvironment.getAsadmin();
 
+    /** Fill up the server log. */
     @BeforeAll
-    public static void fillUpServerLog() {
-        // Fill up the server log.
-        AsadminResult result = ASADMIN.exec("restart-domain", "--timeout", "60");
-        assertThat(result, asadminOK());
+    public static void fillUpServerLog() throws Exception {
+        if (OS.isWindowsForSure()) {
+            // For some reason windows can collide on debug port.
+            assertThat(ASADMIN.exec("stop-domain"), asadminOK());
+            assertThat(ASADMIN.exec("start-domain"), asadminOK());
+        } else {
+            assertThat(ASADMIN.exec("restart-domain"), asadminOK());
+        }
     }
 
     @Test

appserver/tests/admin/tests/src/test/java/org/glassfish/main/admin/test/StartServITest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024 Eclipse Foundation and/or its affiliates. All rights reserved.
+ * Copyright (c) 2024, 2025 Contributors to the Eclipse Foundation.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0, which is available at
@@ -13,6 +13,7 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  */
+
 package org.glassfish.main.admin.test;
 
 import java.util.stream.Stream;
@@ -22,6 +23,7 @@
 import org.glassfish.main.itest.tools.asadmin.AsadminResult;
 import org.glassfish.main.itest.tools.asadmin.StartServ;
 import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.condition.DisabledOnOs;
 import org.junit.jupiter.api.extension.ExtensionContext;
@@ -54,29 +56,34 @@ static void setupDomain() {
         }
     }
 
+    @AfterEach
+    void stopDomain() {
+        ASADMIN.exec("stop-domain", STARTSERV_DOMAIN_NAME);
+    }
+
+    @AfterAll
+    static void deleteDomain() {
+        AsadminResult result = ASADMIN.exec("list-domains");
+        if (result.getOutput().contains(STARTSERV_DOMAIN_NAME)) {
+            ASADMIN.exec("delete-domain", STARTSERV_DOMAIN_NAME);
+        }
+    }
+
     @ParameterizedTest
     @ArgumentsSource(StartServArgumentsProvider.class)
     public void startServerInForeground(StartServ startServ) {
-        try {
-            AsadminResult result = startServ.withTextToWaitFor("Total startup time including CLI").exec(STARTSERV_DOMAIN_NAME);
-            assertThat(result, asadminOK());
-        } finally {
-            ASADMIN.exec("stop-domain", STARTSERV_DOMAIN_NAME);
-        }
+        AsadminResult result = startServ.withTextToWaitFor("Total startup time including CLI").exec(STARTSERV_DOMAIN_NAME);
+        assertThat(result, asadminOK());
     }
 
     @ParameterizedTest
     @ArgumentsSource(StartServArgumentsProvider.class)
     @DisabledOnOs(value = WINDOWS, disabledReason = "startserv.bat is just trivial and doesn't give the error output")
     public void reportCorrectErrorIfAlreadyRunning(StartServ startServ) {
-        try {
-            AsadminResult result = startServ.withTextToWaitFor("Total startup time including CLI").exec(STARTSERV_DOMAIN_NAME);
-            assertThat(result, asadminOK());
-            result = startServ.withNoTextToWaitFor().exec(STARTSERV_DOMAIN_NAME);
-            assertThat(result.getStdErr(), containsString("There is a process already using the admin port"));
-        } finally {
-            ASADMIN.exec("stop-domain", STARTSERV_DOMAIN_NAME);
-        }
+        AsadminResult result = startServ.withTextToWaitFor("Total startup time including CLI").exec(STARTSERV_DOMAIN_NAME);
+        assertThat(result, asadminOK());
+        result = startServ.withNoTextToWaitFor().exec(STARTSERV_DOMAIN_NAME);
+        assertThat(result.getStdErr(), containsString("There is a process already using the admin port"));
     }
 
     @ParameterizedTest
@@ -86,14 +93,6 @@ public void reportCorrectErrorIfInvalidCommand(StartServ startServ) {
         assertThat(result.getStdErr(), containsString("There is no such domain directory"));
     }
 
-    @AfterAll
-    static void deleteDomain() {
-        AsadminResult result = ASADMIN.exec("list-domains");
-        if (result.getOutput().contains(STARTSERV_DOMAIN_NAME)) {
-            ASADMIN.exec("delete-domain", STARTSERV_DOMAIN_NAME);
-        }
-    }
-
     static class StartServArgumentsProvider implements ArgumentsProvider {
 
         @Override

appserver/tests/admin/tests/src/test/java/org/glassfish/main/admin/test/progress/JobManagerITest.java

@@ -63,7 +63,7 @@ public void jobSurvivesRestart() throws Exception {
             "--cleanup-initial-delay=0s",
             "--cleanup-poll-interval=0s"), asadminOK());
         assertThat(ASADMIN.exec(COMMAND_PROGRESS_SIMPLE), asadminOK());
-        assertThat(ASADMIN.exec("restart-domain", "--timeout", "60"), asadminOK());
+        assertThat(ASADMIN.exec("restart-domain", "--timeout", "60", "domain1"), asadminOK());
         assertThat(ASADMIN.exec("list-jobs").getStdOut(), stringContainsInOrder(COMMAND_PROGRESS_SIMPLE, "COMPLETED"));
         JobTestExtension.doAndDisableJobCleanup();
     }