Skip to content

Commit

Permalink
Issue #24228 Removed redundant pid file removal, faster checking
Browse files Browse the repository at this point in the history
- The file is removed twice while it is used to check if the instance is still
  running.
- faster check to increase probability of issues if there are some - and also
  to be really faster :-)
- Local testing: start: 2200-2500 ms, stop: 30-40 ms.
- Added log for failures.
- The javadoc was not right, it is not possible in current Java versions with
  the current GlassFish implementation, but it is possible to avoid hooks by
  using SIGKILL signal. But that isn't an issue any more, because since GF7
  we use ProcessHandles to check if the process is running, while old versions
  used commands of the operating system.
- However, I am still not sure if this is enough to resolve the issue, because
  Felix really writes to the osgi cache until the real end. But JVM hooks are
  executed AFTER parallely executed application hooks and the DeleteOnExitHook
  should be the last one. That should remove the pid file.
  Then asadmin command checks if it exists or not, if it exists, it loads it's
  content and checks handles if the process is alive or not.
  On startup it checks the same, so there is no problem.
- Tested CTRL+C, SIGKILL, pid, stop, --force, --kill, restart-domain, everything
  works now.

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
  • Loading branch information
dmatej committed Jan 17, 2023
1 parent 71d06d5 commit 2715483
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 42 deletions.
@@ -1,4 +1,5 @@
/*
* Copyright (c) 2023 Contributors to the Eclipse Foundation
* Copyright (c) 2008, 2018 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
Expand All @@ -16,17 +17,24 @@

package com.sun.enterprise.v3.admin.cluster;


import com.sun.enterprise.config.serverbeans.Domain;
import com.sun.enterprise.v3.admin.StopServer;
import org.glassfish.api.Async;
import org.glassfish.api.Param;
import org.glassfish.api.admin.*;

import jakarta.inject.Inject;

import org.jvnet.hk2.annotations.Service;
import org.glassfish.api.Async;
import org.glassfish.api.Param;
import org.glassfish.api.admin.AdminCommand;
import org.glassfish.api.admin.AdminCommandContext;
import org.glassfish.api.admin.CommandLock;
import org.glassfish.api.admin.ExecuteOn;
import org.glassfish.api.admin.RestEndpoint;
import org.glassfish.api.admin.RestEndpoints;
import org.glassfish.api.admin.RuntimeType;
import org.glassfish.api.admin.ServerEnvironment;
import org.glassfish.hk2.api.PerLookup;
import org.glassfish.hk2.api.ServiceLocator;
import org.jvnet.hk2.annotations.Service;

/**
* AdminCommand to stop the instance
Expand Down Expand Up @@ -59,6 +67,7 @@ public class StopInstanceInstanceCommand extends StopServer implements AdminComm
@Param(optional = true, defaultValue = "true")
private Boolean force = true;

@Override
public void execute(AdminCommandContext context) {

if (!env.isInstance()) {
Expand Down
@@ -1,4 +1,5 @@
/*
* Copyright (c) 2023 Contributors to the Eclipse Foundation
* Copyright (c) 2008, 2018 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
Expand All @@ -16,17 +17,23 @@

package com.sun.enterprise.v3.admin;

import com.sun.enterprise.module.ModulesRegistry;
import com.sun.enterprise.util.LocalStringManagerImpl;

import jakarta.inject.Inject;

import org.glassfish.api.Async;
import org.glassfish.api.I18n;
import org.glassfish.api.Param;
import org.glassfish.api.admin.*;
import jakarta.inject.Inject;
import org.jvnet.hk2.annotations.Service;

import org.glassfish.api.admin.AccessRequired;
import org.glassfish.api.admin.AdminCommand;
import org.glassfish.api.admin.AdminCommandContext;
import org.glassfish.api.admin.CommandLock;
import org.glassfish.api.admin.ExecuteOn;
import org.glassfish.api.admin.RuntimeType;
import org.glassfish.api.admin.ServerEnvironment;
import org.glassfish.hk2.api.PerLookup;
import org.glassfish.hk2.api.ServiceLocator;
import org.jvnet.hk2.annotations.Service;

/**
* AdminCommand to stop the domain execution which mean shuting down the application
Expand Down Expand Up @@ -57,6 +64,7 @@ public class StopDomainCommand extends StopServer implements AdminCommand {
* All running services are stopped.
* LookupManager is flushed.
*/
@Override
public void execute(AdminCommandContext context) {

if (!env.isDas()) {
Expand Down
@@ -1,4 +1,5 @@
/*
* Copyright (c) 2023 Contributors to the Eclipse Foundation
* Copyright (c) 2008, 2018 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
Expand All @@ -16,25 +17,26 @@

package com.sun.enterprise.v3.admin;

import com.sun.enterprise.util.LocalStringManagerImpl;
import com.sun.enterprise.util.io.FileUtils;
import java.io.File;
import java.util.logging.Level;

import org.glassfish.api.admin.ServerEnvironment;
import org.glassfish.embeddable.GlassFish;
import org.glassfish.hk2.api.ServiceLocator;
import org.glassfish.kernel.KernelLoggerInfo;

/**
* A class to house identical code for stopping instances and DAS
*
* @author Byron Nevins
*/
public class StopServer {

/**
* Shutdown of the server :
*
* All running services are stopped.
* LookupManager is flushed.
* Shutdown of the server:
* <ol>
* <li>All running services are stopped.
* <li>LookupManager is flushed.
* </ol>
*/
protected final void doExecute(ServiceLocator habitat, ServerEnvironment env, boolean force) {
try {
Expand All @@ -48,39 +50,17 @@ protected final void doExecute(ServiceLocator habitat, ServerEnvironment env, bo
// show up in the ServiceLocator.
GlassFish gfKernel = habitat.getService(GlassFish.class);
while (gfKernel == null) {
Thread.sleep(1000);
Thread.yield();
gfKernel = habitat.getService(GlassFish.class);
}
// gfKernel is absolutely positively for-sure not null.
gfKernel.stop();
}
catch (Throwable t) {
// ignore
} catch (Throwable t) {
KernelLoggerInfo.getLogger().log(Level.WARNING, "Server kernel stop failed.", t);
}


if (force) {
System.exit(0);
}
else {
deletePidFile(env);
}
}

private final static LocalStringManagerImpl localStrings = new LocalStringManagerImpl(StopServer.class);

/**
* It is **Essential** to delete this file! Other code will assume the server
* is running if it exists.
* Any old App is currently (10/10/10) allowed to add a shutdownhook with a System.exit()
* which is GUARANTEED to prevent the shutdown hook for deleting the pidfile to run.
* So -- we always do it BEFORE trying to exit.
*/
private void deletePidFile(ServerEnvironment env) {
File pidFile = new File(env.getConfigDirPath(), "pid");

if (pidFile.isFile()) {
FileUtils.deleteFileNowOrLater(pidFile);
}
}
}

0 comments on commit 2715483

Please sign in to comment.