From 3207e1130e128102d148df3b2e7a9b9b98ae5452 Mon Sep 17 00:00:00 2001 From: merorai Date: Thu, 4 Jun 2020 15:15:02 +0100 Subject: [PATCH 1/3] CUSTCOM-14 Improvements in stop-domain process --- .../servermgmt/cli/LocalServerCommand.java | 25 ++++++-------- .../servermgmt/cli/StopDomainCommand.java | 3 +- .../v3/server/AppServerStartup.java | 34 +++++++++++++++++++ 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/LocalServerCommand.java b/nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/LocalServerCommand.java index 90321d6185a..82d8b2a584c 100644 --- a/nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/LocalServerCommand.java +++ b/nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/LocalServerCommand.java @@ -66,6 +66,8 @@ import com.sun.enterprise.util.HostAndPort; import com.sun.enterprise.util.io.FileUtils; import com.sun.enterprise.util.io.ServerDirs; +import java.net.InetAddress; +import java.net.InetSocketAddress; import org.glassfish.api.ActionReport; import org.glassfish.api.admin.CommandException; @@ -94,6 +96,7 @@ public abstract class LocalServerCommand extends CLICommand { //////////////////////////////////////////////////////////////// private ServerDirs serverDirs; private static final LocalStringsImpl STRINGS = new LocalStringsImpl(LocalDomainCommand.class); + private final static int IS_RUNNING_DEFAULT_TIMEOUT = 2000; //////////////////////////////////////////////////////////////// /// Section: protected variables @@ -354,24 +357,18 @@ protected final int getServerPid() { * @return boolean indicating whether the server is running */ protected final boolean isRunning(String host, int port) { - Socket server = null; - try { - server = new Socket(host, port); + + try(Socket server = new Socket()) { + if (host == null) { + host = InetAddress.getByName(null).getHostName(); + } + + server.connect( new InetSocketAddress(host, port), IS_RUNNING_DEFAULT_TIMEOUT); return true; - } - catch (Exception ex) { + }catch (Exception ex) { logger.log(Level.FINER, "\nisRunning got exception: {0}", ex); return false; } - finally { - if (server != null) { - try { - server.close(); - } - catch (IOException ex) { - } - } - } } /** diff --git a/nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/StopDomainCommand.java b/nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/StopDomainCommand.java index 03bdbfef5db..6ee81cec851 100644 --- a/nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/StopDomainCommand.java +++ b/nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/StopDomainCommand.java @@ -46,6 +46,7 @@ import com.sun.enterprise.admin.cli.remote.RemoteCLICommand; import com.sun.enterprise.universal.process.ProcessUtils; import com.sun.enterprise.util.io.FileUtils; +import com.sun.enterprise.util.net.NetUtils; import java.io.File; import java.util.Map; import java.util.Map.Entry; @@ -98,7 +99,7 @@ protected void initDomain() throws CommandException { // TODO Byron said in April 2013 that we should probably just check if // NetUtils says that the getHost() --> isThisMe() rather than merely // checking for the magic "localhost" string. Too risky to fool with it today. - if (programOpts.getHost().equals(CLIConstants.DEFAULT_HOSTNAME)) { + if (NetUtils.isThisHostLocal(programOpts.getHost())) { super.initDomain(); } else if (userArgDomainName != null) { // remote case throw new CommandException(Strings.get("StopDomain.noDomainNameAllowed")); diff --git a/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/AppServerStartup.java b/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/AppServerStartup.java index fe714861497..3f9a6974ece 100644 --- a/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/AppServerStartup.java +++ b/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/AppServerStartup.java @@ -51,6 +51,9 @@ import com.sun.enterprise.module.bootstrap.StartupContext; import com.sun.enterprise.util.Result; import com.sun.enterprise.admin.report.DoNothingActionReporter; +import com.sun.enterprise.universal.process.Jps; +import java.io.BufferedReader; +import java.io.InputStreamReader; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.LinkedList; @@ -504,6 +507,21 @@ public synchronized void stop() { } catch (Exception e) { logger.log(Level.SEVERE, KernelLoggerInfo.exceptionDuringShutdown, e); } + + String s; + Process p; + try { + p = Runtime.getRuntime().exec("netstat -plten |grep java"); + BufferedReader br = new BufferedReader( + new InputStreamReader(p.getInputStream())); + while ((s = br.readLine()) != null) { + System.out.println("line: " + s); + } + p.waitFor(); + System.out.println("exit: " + p.exitValue()); + p.destroy(); + } catch (Exception e) { + } // deactivate the run level services try { @@ -512,6 +530,22 @@ public synchronized void stop() { logger.log(Level.SEVERE, KernelLoggerInfo.exceptionDuringShutdown, e); } + + try { + p = Runtime.getRuntime().exec("netstat -plten |grep java"); + BufferedReader br = new BufferedReader( + new InputStreamReader(p.getInputStream())); + while ((s = br.readLine()) != null) { + System.out.println("line: " + s); + } + p.waitFor(); + System.out.println("exit: " + p.exitValue()); + p.destroy(); + } catch (Exception e) { + } + + + System.out.println("jps id is ================================ " + Jps.getPid("ASMain")); // first send the shutdown event synchronously env.setStatus(ServerEnvironment.Status.stopped); try { From a32d0a75bbedfba50b1650b180bce0c79b002da0 Mon Sep 17 00:00:00 2001 From: merorai Date: Thu, 4 Jun 2020 15:25:10 +0100 Subject: [PATCH 2/3] Removed debugging code --- .../v3/server/AppServerStartup.java | 99 +++++++------------ 1 file changed, 34 insertions(+), 65 deletions(-) diff --git a/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/AppServerStartup.java b/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/AppServerStartup.java index 3f9a6974ece..ef15dbda1a8 100644 --- a/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/AppServerStartup.java +++ b/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/AppServerStartup.java @@ -51,9 +51,6 @@ import com.sun.enterprise.module.bootstrap.StartupContext; import com.sun.enterprise.util.Result; import com.sun.enterprise.admin.report.DoNothingActionReporter; -import com.sun.enterprise.universal.process.Jps; -import java.io.BufferedReader; -import java.io.InputStreamReader; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.LinkedList; @@ -63,7 +60,6 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; import javax.inject.Inject; @@ -117,9 +113,6 @@ @Service @Rank(Constants.DEFAULT_IMPLEMENTATION_RANK) // This should be the default impl if no name is specified public class AppServerStartup implements PostConstruct, ModuleStartup { - enum State { - INITIAL, STARTING, STARTED, SHUTDOWN_REQUESTED, SHUTTING_DOWN, SHUT_DOWN; - } StartupContext context; @@ -136,12 +129,14 @@ enum State { @Inject ModulesRegistry systemRegistry; - @Override @Inject public void setStartupContext(StartupContext context) { this.context = context; } + @Inject + ExecutorService executor; + @Inject Events events; @@ -171,7 +166,7 @@ public void setStartupContext(StartupContext context) { * as long as GlassFish kernel is up. */ private Thread serverThread; - private AtomicReference state = new AtomicReference<>(State.INITIAL); + private boolean shutdownSignal = false; private final static String THREAD_POLICY_PROPERTY = "org.glassfish.startupThreadPolicy"; private final static String MAX_STARTUP_THREAD_PROPERTY = "org.glassfish.maxStartupThreads"; @@ -225,18 +220,10 @@ public synchronized void start() { // See issue #5596 to know why we set context CL as common CL. Thread.currentThread().setContextClassLoader( commonCLS.getCommonClassLoader()); - if (state.compareAndSet(State.INITIAL, State.STARTING) || state.compareAndSet(State.SHUT_DOWN, State.STARTING)) { - doStart(); - } else { - throw new GlassFishException("Server cannot start, because it's in state "+state.get()); - } + doStart(); } catch (GlassFishException ex) { throw new RuntimeException (ex); } finally { - if (!state.compareAndSet(State.STARTING, State.STARTED)) { - // the state is no longer STARTING, therefore it has to be SHUTDOWN_REQUESTED - stop(); - } // reset the context classloader. See issue GLASSFISH-15775 Thread.currentThread().setContextClassLoader(origCL); } @@ -267,7 +254,7 @@ public void run() { latch.countDown(); synchronized (this) { - while (state.get() != AppServerStartup.State.SHUT_DOWN ) { + while (!shutdownSignal) { try { wait(); // Wait indefinitely until shutdown is requested } @@ -428,6 +415,7 @@ private boolean postStartupJob() { if (future.get(3, TimeUnit.SECONDS).isFailure()) { final Throwable t = future.get().exception(); logger.log(Level.SEVERE, KernelLoggerInfo.startupFatalException, t); + events.send(new Event(EventTypes.SERVER_SHUTDOWN), false); shutdown(); return false; } @@ -478,23 +466,31 @@ public static void printModuleStatus(ModulesRegistry registry, Level level) { logger.log(level, sb.toString()); } + // TODO(Sahoo): Revisit this method after discussing with Jerome. private void shutdown() { - if ( state.compareAndSet(State.STARTING, State.SHUTDOWN_REQUESTED) || - state.compareAndSet(State.INITIAL, State.SHUT_DOWN)) { - // SHUTDOWN_REQUESTED is handled at end of START - // INITIAL --> SHUT_DOWN is trivial case of shutdown before we started - return; - } - // state doesn't change on SHUT_DOWN, SHUTTING_DOWN and SHUTDOWN_REQUESTED - if (state.compareAndSet(State.STARTED, State.SHUTDOWN_REQUESTED)) { - // directly stop only if server already completed startup sequence - stop(); + CommandRunner runner = commandRunnerProvider.get(); + + if (runner!=null) { + final ParameterMap params = new ParameterMap(); + // By default we don't want to shutdown forcefully, as that will cause the VM to exit and that's not + // a very good behavior for a code known to be embedded in other processes. + final boolean noForcedShutdown = + Boolean.parseBoolean(context.getArguments().getProperty( + com.sun.enterprise.glassfish.bootstrap.Constants.NO_FORCED_SHUTDOWN, "true")); + if (noForcedShutdown) { + params.set("force", "false"); + } + final InternalSystemAdministrator kernelIdentity = locator.getService(InternalSystemAdministrator.class); + if (env.isDas()) { + runner.getCommandInvocation("stop-domain", new DoNothingActionReporter(), kernelIdentity.getSubject()).parameters(params).execute(); + } else { + runner.getCommandInvocation("_stop-instance", new DoNothingActionReporter(), kernelIdentity.getSubject()).parameters(params).execute(); + } } } - @Override + @SuppressWarnings({ "rawtypes", "unchecked" }) public synchronized void stop() { - state.set(State.SHUTTING_DOWN); if(env.getStatus() == ServerEnvironment.Status.stopped) { // During shutdown because of shutdown hooks, we can be stopped multiple times. // In such a case, ignore any subsequent stop operations. @@ -507,21 +503,6 @@ public synchronized void stop() { } catch (Exception e) { logger.log(Level.SEVERE, KernelLoggerInfo.exceptionDuringShutdown, e); } - - String s; - Process p; - try { - p = Runtime.getRuntime().exec("netstat -plten |grep java"); - BufferedReader br = new BufferedReader( - new InputStreamReader(p.getInputStream())); - while ((s = br.readLine()) != null) { - System.out.println("line: " + s); - } - p.waitFor(); - System.out.println("exit: " + p.exitValue()); - p.destroy(); - } catch (Exception e) { - } // deactivate the run level services try { @@ -530,22 +511,6 @@ public synchronized void stop() { logger.log(Level.SEVERE, KernelLoggerInfo.exceptionDuringShutdown, e); } - - try { - p = Runtime.getRuntime().exec("netstat -plten |grep java"); - BufferedReader br = new BufferedReader( - new InputStreamReader(p.getInputStream())); - while ((s = br.readLine()) != null) { - System.out.println("line: " + s); - } - p.waitFor(); - System.out.println("exit: " + p.exitValue()); - p.destroy(); - } catch (Exception e) { - } - - - System.out.println("jps id is ================================ " + Jps.getPid("ASMain")); // first send the shutdown event synchronously env.setStatus(ServerEnvironment.Status.stopped); try { @@ -562,10 +527,11 @@ public synchronized void stop() { logger.info(KernelLoggerInfo.shutdownFinished); - state.set(State.SHUT_DOWN); // notify the server thread that we are done, so that it can come out. if (serverThread!=null) { synchronized (serverThread) { + shutdownSignal = true; + serverThread.notify(); } try { @@ -577,9 +543,11 @@ public synchronized void stop() { } /** - * Proceed to the given run level. + * Proceed to the given run level using the given {@link AppServerActivator}. * * @param runLevel the run level to proceed to + * @param activator an {@link AppServerActivator activator} used to + * activate/deactivate the services * @return false if an error occurred that required server shutdown; true otherwise */ private boolean proceedTo(int runLevel) { @@ -726,7 +694,7 @@ private LinkedHashMap getAllRecordedTimes() { private class MasterRunLevelListener implements RunLevelListener { private final RunLevelController controller; - private volatile boolean forcedShutdown = false; + private boolean forcedShutdown = false; private MasterRunLevelListener(RunLevelController controller) { this.controller = controller; @@ -761,6 +729,7 @@ public void onError(RunLevelFuture future, ErrorInformation info) { if (controller.getCurrentRunLevel() >= InitRunLevel.VAL) { logger.log(Level.SEVERE, KernelLoggerInfo.startupFailure, info.getError()); + events.send(new Event(EventTypes.SERVER_SHUTDOWN), false); } forcedShutdown = true; From b9a52c2c53767f5a22f459052a6011f353da4ca2 Mon Sep 17 00:00:00 2001 From: merorai Date: Tue, 9 Jun 2020 10:50:50 +0100 Subject: [PATCH 3/3] * Removed obsolete comments * Added back Patrik's commit which was removed by mistake" --- .../servermgmt/cli/StopDomainCommand.java | 3 - .../v3/server/AppServerStartup.java | 65 +++++++++---------- 2 files changed, 31 insertions(+), 37 deletions(-) diff --git a/nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/StopDomainCommand.java b/nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/StopDomainCommand.java index 6ee81cec851..339012af4bf 100644 --- a/nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/StopDomainCommand.java +++ b/nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/StopDomainCommand.java @@ -96,9 +96,6 @@ protected void validate() protected void initDomain() throws CommandException { // only initialize local domain information if it's a local operation - // TODO Byron said in April 2013 that we should probably just check if - // NetUtils says that the getHost() --> isThisMe() rather than merely - // checking for the magic "localhost" string. Too risky to fool with it today. if (NetUtils.isThisHostLocal(programOpts.getHost())) { super.initDomain(); } else if (userArgDomainName != null) { // remote case diff --git a/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/AppServerStartup.java b/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/AppServerStartup.java index ef15dbda1a8..fe714861497 100644 --- a/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/AppServerStartup.java +++ b/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/AppServerStartup.java @@ -60,6 +60,7 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; import javax.inject.Inject; @@ -113,6 +114,9 @@ @Service @Rank(Constants.DEFAULT_IMPLEMENTATION_RANK) // This should be the default impl if no name is specified public class AppServerStartup implements PostConstruct, ModuleStartup { + enum State { + INITIAL, STARTING, STARTED, SHUTDOWN_REQUESTED, SHUTTING_DOWN, SHUT_DOWN; + } StartupContext context; @@ -129,14 +133,12 @@ public class AppServerStartup implements PostConstruct, ModuleStartup { @Inject ModulesRegistry systemRegistry; + @Override @Inject public void setStartupContext(StartupContext context) { this.context = context; } - @Inject - ExecutorService executor; - @Inject Events events; @@ -166,7 +168,7 @@ public void setStartupContext(StartupContext context) { * as long as GlassFish kernel is up. */ private Thread serverThread; - private boolean shutdownSignal = false; + private AtomicReference state = new AtomicReference<>(State.INITIAL); private final static String THREAD_POLICY_PROPERTY = "org.glassfish.startupThreadPolicy"; private final static String MAX_STARTUP_THREAD_PROPERTY = "org.glassfish.maxStartupThreads"; @@ -220,10 +222,18 @@ public synchronized void start() { // See issue #5596 to know why we set context CL as common CL. Thread.currentThread().setContextClassLoader( commonCLS.getCommonClassLoader()); - doStart(); + if (state.compareAndSet(State.INITIAL, State.STARTING) || state.compareAndSet(State.SHUT_DOWN, State.STARTING)) { + doStart(); + } else { + throw new GlassFishException("Server cannot start, because it's in state "+state.get()); + } } catch (GlassFishException ex) { throw new RuntimeException (ex); } finally { + if (!state.compareAndSet(State.STARTING, State.STARTED)) { + // the state is no longer STARTING, therefore it has to be SHUTDOWN_REQUESTED + stop(); + } // reset the context classloader. See issue GLASSFISH-15775 Thread.currentThread().setContextClassLoader(origCL); } @@ -254,7 +264,7 @@ public void run() { latch.countDown(); synchronized (this) { - while (!shutdownSignal) { + while (state.get() != AppServerStartup.State.SHUT_DOWN ) { try { wait(); // Wait indefinitely until shutdown is requested } @@ -415,7 +425,6 @@ private boolean postStartupJob() { if (future.get(3, TimeUnit.SECONDS).isFailure()) { final Throwable t = future.get().exception(); logger.log(Level.SEVERE, KernelLoggerInfo.startupFatalException, t); - events.send(new Event(EventTypes.SERVER_SHUTDOWN), false); shutdown(); return false; } @@ -466,31 +475,23 @@ public static void printModuleStatus(ModulesRegistry registry, Level level) { logger.log(level, sb.toString()); } - // TODO(Sahoo): Revisit this method after discussing with Jerome. private void shutdown() { - CommandRunner runner = commandRunnerProvider.get(); - - if (runner!=null) { - final ParameterMap params = new ParameterMap(); - // By default we don't want to shutdown forcefully, as that will cause the VM to exit and that's not - // a very good behavior for a code known to be embedded in other processes. - final boolean noForcedShutdown = - Boolean.parseBoolean(context.getArguments().getProperty( - com.sun.enterprise.glassfish.bootstrap.Constants.NO_FORCED_SHUTDOWN, "true")); - if (noForcedShutdown) { - params.set("force", "false"); - } - final InternalSystemAdministrator kernelIdentity = locator.getService(InternalSystemAdministrator.class); - if (env.isDas()) { - runner.getCommandInvocation("stop-domain", new DoNothingActionReporter(), kernelIdentity.getSubject()).parameters(params).execute(); - } else { - runner.getCommandInvocation("_stop-instance", new DoNothingActionReporter(), kernelIdentity.getSubject()).parameters(params).execute(); - } + if ( state.compareAndSet(State.STARTING, State.SHUTDOWN_REQUESTED) || + state.compareAndSet(State.INITIAL, State.SHUT_DOWN)) { + // SHUTDOWN_REQUESTED is handled at end of START + // INITIAL --> SHUT_DOWN is trivial case of shutdown before we started + return; + } + // state doesn't change on SHUT_DOWN, SHUTTING_DOWN and SHUTDOWN_REQUESTED + if (state.compareAndSet(State.STARTED, State.SHUTDOWN_REQUESTED)) { + // directly stop only if server already completed startup sequence + stop(); } } - @SuppressWarnings({ "rawtypes", "unchecked" }) + @Override public synchronized void stop() { + state.set(State.SHUTTING_DOWN); if(env.getStatus() == ServerEnvironment.Status.stopped) { // During shutdown because of shutdown hooks, we can be stopped multiple times. // In such a case, ignore any subsequent stop operations. @@ -527,11 +528,10 @@ public synchronized void stop() { logger.info(KernelLoggerInfo.shutdownFinished); + state.set(State.SHUT_DOWN); // notify the server thread that we are done, so that it can come out. if (serverThread!=null) { synchronized (serverThread) { - shutdownSignal = true; - serverThread.notify(); } try { @@ -543,11 +543,9 @@ public synchronized void stop() { } /** - * Proceed to the given run level using the given {@link AppServerActivator}. + * Proceed to the given run level. * * @param runLevel the run level to proceed to - * @param activator an {@link AppServerActivator activator} used to - * activate/deactivate the services * @return false if an error occurred that required server shutdown; true otherwise */ private boolean proceedTo(int runLevel) { @@ -694,7 +692,7 @@ private LinkedHashMap getAllRecordedTimes() { private class MasterRunLevelListener implements RunLevelListener { private final RunLevelController controller; - private boolean forcedShutdown = false; + private volatile boolean forcedShutdown = false; private MasterRunLevelListener(RunLevelController controller) { this.controller = controller; @@ -729,7 +727,6 @@ public void onError(RunLevelFuture future, ErrorInformation info) { if (controller.getCurrentRunLevel() >= InitRunLevel.VAL) { logger.log(Level.SEVERE, KernelLoggerInfo.startupFailure, info.getError()); - events.send(new Event(EventTypes.SERVER_SHUTDOWN), false); } forcedShutdown = true;