Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CUSTCOM-14 Improvements in stop-domain process #4699

Merged
merged 3 commits into from
Jun 24, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
}
}
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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())) {
MeroRai marked this conversation as resolved.
Show resolved Hide resolved
super.initDomain();
} else if (userArgDomainName != null) { // remote case
throw new CommandException(Strings.get("StopDomain.noDomainNameAllowed"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,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;
Expand Down Expand Up @@ -114,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;

Expand All @@ -133,12 +129,14 @@ enum State {
@Inject
ModulesRegistry systemRegistry;

@Override
@Inject
public void setStartupContext(StartupContext context) {
this.context = context;
}

@Inject
ExecutorService executor;

@Inject
Events events;

Expand Down Expand Up @@ -168,7 +166,7 @@ public void setStartupContext(StartupContext context) {
* as long as GlassFish kernel is up.
*/
private Thread serverThread;
private AtomicReference<State> 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";
Expand Down Expand Up @@ -222,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)) {
MeroRai marked this conversation as resolved.
Show resolved Hide resolved
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);
}
Expand Down Expand Up @@ -264,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
}
Expand Down Expand Up @@ -425,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;
}
Expand Down Expand Up @@ -475,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.
MeroRai marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down Expand Up @@ -528,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 {
Expand All @@ -543,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) {
MeroRai marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -692,7 +694,7 @@ private LinkedHashMap<String, Long> 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;
Expand Down Expand Up @@ -727,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;
Expand Down