Skip to content

[WFCORE-6934] Server startup marker file is not a reliable method to detect when the server is running (POC) #6756

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.

Sign up for GitHub

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

Open
elguardian wants to merge 7 commits into
base: main
Choose a base branch
from
Open

[WFCORE-6934] Server startup marker file is not a reliable method to detect when the server is running (POC) #6756

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
69bbe94
[WFCORE-6934] Server startup marker file is not a reliable method to …
elguardian Jun 9, 2026
e1f5fc8
fix for respawn tests
elguardian Jun 15, 2026
18e8429
added yersen feedback
elguardian Jun 16, 2026
1ffe8ea
add feedback from yeray in zulip
elguardian Aug 27, 2026
1a341e3
remove indirection
elguardian Aug 27, 2026
e534e98
remove bootstrap listener from staging changes
elguardian Aug 27, 2026
c773692
add test case for windows
elguardian Aug 27, 2026
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
25 changes: 25 additions & 0 deletions process-controller/src/main/java/org/jboss/as/process/Main.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
import java.io.IOException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.nio.channels.FileChannel;
import java.nio.channels.FileLock;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
import java.security.PrivilegedAction;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -201,6 +206,9 @@ public JBossThreadFactory run() {
configuration.setReadExecutor(Executors.newCachedThreadPool(threadFactory));

final ProcessController processController = new ProcessController(configuration, System.out, System.err);

processController.setRunningLockChannel(acquireRunningLock(jbossHome));
Comment thread
yersan marked this conversation as resolved.
Outdated

final InetSocketAddress boundAddress = processController.getServer().getBoundAddress();

final List<String> initialCommand = new ArrayList<String>();
Expand Down Expand Up @@ -243,6 +251,23 @@ public void run() {
return processController;
}

private static FileChannel acquireRunningLock(String jbossHome) {
try {
Path lockPath = Paths.get(jbossHome, ".installation", "running.lock");
java.nio.file.Files.createDirectories(lockPath.getParent());
Comment thread
yersan marked this conversation as resolved.
Outdated
FileChannel channel = FileChannel.open(lockPath,
StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.READ);
FileLock lock = channel.tryLock();
if (lock != null) {
return channel;
}

@yersan yersan Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to decide what we want to do in this case.

If the lock is not acquired, what does it mean? Do we want to start the server as usual? (I would say yes) If so, Prospero should not try to upgrade the server since we will never know whether the lock was legitimately acquired to signal that the server is running.

We need to sort out this case and control it. If the server does not acquired the lock, by whatever reason, how Prospero would understand whether the server is still running?

It seems to me we need a fallback for this case. That fallback case can be the old which uses temp directory, although moving it out of the temp directory. How to indicate to Prospero whether the server has legitimately acquired the log is still open. As first glance it could be a flag on the Installation Manager service that is written down into the properties file used to communicate to Prospero any context for the apply operation.

@elguardian elguardian Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this just is for a test.... jvm has some sort of reentry for locks so it does not allow to acquire the lock twice (make sense), so if the lock is already taken... it should be by this jvm.

@yersan yersan Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, yes, we should not have other JVMs trying to get a lock ther .... just keep an eye on the embedded server / embedded Host Controller, I guess they won't start from main methods .. but just in case

channel.close();
} catch (IOException e) {
// ignore
}
return null;
}

private static boolean isJavaSecurityManagerConfigured(final String arg) {
return arg.startsWith("-Djava.security.manager")
&& !"-Djava.security.manager=allow".equals(arg)
Expand Down
15 changes: 15 additions & 0 deletions process-controller/src/main/java/org/jboss/as/process/ProcessController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.io.PrintStream;
import java.nio.channels.FileChannel;
import java.nio.charset.StandardCharsets;
import java.security.SecureRandom;
import java.util.Arrays;
Expand Down Expand Up @@ -46,6 +47,7 @@ public final class ProcessController {
private final Set<Connection> managedConnections = new CopyOnWriteArraySet<Connection>();

private volatile boolean shutdown;
private volatile FileChannel runningLockChannel;

private static final short AUTH_BYTES_LENGTH = 16;
public static final short AUTH_BYTES_ENCODED_LENGTH = 24;
Expand Down Expand Up @@ -224,9 +226,22 @@ public void shutdown() {
}
}
ProcessLogger.ROOT_LOGGER.shutdownComplete();
final FileChannel channel = runningLockChannel;
if (channel != null) {
runningLockChannel = null;
try {
channel.close();
} catch (IOException e) {
// ignore

@yersan yersan Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not silently ignore them, it could help on debugging, could we at least add a debug trace?

ProcessLogger.ROOT_LOGGER.debugf("Failed to close process controller lock channel");

Something like this.

}
}
}
}

void setRunningLockChannel(FileChannel channel) {
this.runningLockChannel = channel;
}

public ManagedProcess getServerByAuthCode(final byte[] code) {
synchronized (lock) {
return processesByKey.get(new ProcessControllerKey(code));
Expand Down
11 changes: 9 additions & 2 deletions server/src/main/java/org/jboss/as/server/ApplicationServerService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ final class ApplicationServerService implements Service<AsyncFuture<ServiceConta
private final ElapsedTime elapsedTime;
private volatile FutureServiceContainer futureContainer;
private volatile boolean everStopped;
private volatile BootstrapListener bootstrapListener;
private static final boolean IGNORE_ROOT_USERNAME_WARN = Boolean.getBoolean("jboss.ignore.root.username.warning");

ApplicationServerService(final List<ServiceActivator> extraServices, final Bootstrap.Configuration configuration,
Expand Down Expand Up @@ -135,8 +136,11 @@ public synchronized void start(final StartContext context) throws StartException

CurrentServiceContainer.setServiceContainer(context.getController().getServiceContainer());

final BootstrapListener bootstrapListener = new BootstrapListener(container, startupTime, serviceTarget, futureContainer, prettyVersion, serverEnvironment.getServerTempDir());
bootstrapListener.getStabilityMonitor().addController(myController);
this.bootstrapListener = new BootstrapListener(container, startupTime, serviceTarget, futureContainer, prettyVersion, serverEnvironment.getServerTempDir());
this.bootstrapListener.getStabilityMonitor().addController(myController);
if (serverEnvironment.getLaunchType() != ServerEnvironment.LaunchType.EMBEDDED) {
this.bootstrapListener.acquireRunningLock(serverEnvironment.getHomeDir());
}
// Install either a local or remote content repository
if(standalone) {
if ( ! selfContained ) {
Expand Down Expand Up @@ -205,6 +209,9 @@ public synchronized void stop(final StopContext context) {
CurrentServiceContainer.setServiceContainer(null);
String prettyVersion = configuration.getServerEnvironment().getProductConfig().getPrettyVersionString();
ServerLogger.AS_ROOT_LOGGER.serverStopped(prettyVersion, (int) (context.getElapsedTime() / 1000000L));
if (this.bootstrapListener != null) {
this.bootstrapListener.releaseRunningLock();
}
BootstrapListener.deleteStartupMarker(configuration.getServerEnvironment().getServerTempDir());
everStopped = true;
}
Expand Down
59 changes: 56 additions & 3 deletions server/src/main/java/org/jboss/as/server/BootstrapListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,21 @@
*/
package org.jboss.as.server;

import static java.security.AccessController.doPrivileged;

import java.io.BufferedWriter;
import java.io.File;
import java.io.IOException;
import java.nio.channels.FileChannel;
import java.nio.channels.FileLock;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;

import org.wildfly.security.manager.WildFlySecurityManager;

import org.jboss.as.network.NetworkUtils;
import org.jboss.as.server.logging.ServerLogger;
Expand All @@ -28,6 +37,8 @@
public final class BootstrapListener {

public static final String MARKER_FILE = "startup-marker";
public static final String RUNNING_LOCK_FILE = "running.lock";
private static final String INSTALLATION_DIR = ".installation";

private final StabilityMonitor monitor = new StabilityMonitor();
private final ServiceContainer serviceContainer;
Expand All @@ -36,8 +47,10 @@ public final class BootstrapListener {
private final String prettyVersion;
private final FutureServiceContainer futureContainer;
private final File tempDir;
private String startedCleanMessage;
private String startedWitErrorsMessage;
private String startedCleanMessage;
private String startedWitErrorsMessage;
private volatile FileChannel lockFileChannel;

@yersan yersan Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is still an unnecessary indirection between the BootstrapListener and the ApplicationServerService for this.

From my point if view, changes could be simplified if all the logic to acquire / release the lock is in the ApplicationServerService. Right now the ApplicationServerService uses the BootstrapListener for acquire/releasing the lock on each server start/reload and looks unnecessary.

This is still a risk since during the reload, the advisory lock will be released, meanwhile the server is still started.

private volatile FileLock runningLock;

public BootstrapListener(final ServiceContainer serviceContainer, final ElapsedTime elapsedTime, final ServiceTarget serviceTarget, final FutureServiceContainer futureContainer, final String prettyVersion, final File tempDir) {
this.serviceContainer = serviceContainer;
Expand Down Expand Up @@ -130,7 +143,6 @@ private void createStartupMarker(String result, long startTime) {
} catch (IOException e) {
// ignore
}

}

public static void deleteStartupMarker(File tempDir) {
Expand All @@ -142,6 +154,47 @@ public static void deleteStartupMarker(File tempDir) {
}
}

public void acquireRunningLock(File homeDir) {
final Path lockPath = homeDir.toPath().resolve(INSTALLATION_DIR).resolve(RUNNING_LOCK_FILE);
try {
if (WildFlySecurityManager.isChecking()) {
doPrivileged((PrivilegedExceptionAction<Void>) () -> {
openLockFile(lockPath);
return null;
});
} else {
openLockFile(lockPath);
}
} catch (PrivilegedActionException | IOException e) {

@yersan yersan Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer use security manager, so we do not need to check for permissions.

// ignore
}
}

private void openLockFile(Path lockPath) throws IOException {
Files.createDirectories(lockPath.getParent());
FileChannel channel = FileChannel.open(lockPath,
StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.READ);
FileLock lock = channel.tryLock();
if (lock != null) {
lockFileChannel = channel;
runningLock = lock;
} else {
channel.close();
}
}

public void releaseRunningLock() {
if (lockFileChannel != null) {
try {
lockFileChannel.close();
} catch (IOException e) {
// ignore

@yersan yersan Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not silently ignore it

}
lockFileChannel = null;
runningLock = null;
}
}

public void logAdminConsole() {
ServiceController<?> controller = serviceContainer.getService(UndertowHttpManagementService.SERVICE_NAME);
if (controller != null) {
Expand Down
Loading