From 2210a0de16da5c6bde80893e61725ec23c3ac36b Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Tue, 28 Jul 2026 14:25:48 +0000
Subject: [PATCH] [#768] Fix winlauncher.exe stop reporting success without stopping the server (#772)
---
opendj-server-legacy/src/build-tools/windows/winlauncher.c | 145 +++++++++++++++++++++++++++++++++++++++++-------
opendj-server-legacy/resource/bin/stop-ds.bat | 10 +++
.github/workflows/build.yml | 19 ++++++
opendj-server-legacy/src/build-tools/windows/winlauncher.h | 5 +
4 files changed, 157 insertions(+), 22 deletions(-)
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 8691a0e..1767f9a 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -287,6 +287,21 @@
- name: Test on Windows
if: runner.os == 'Windows'
run: |
+ # Verify a stop took effect before moving on: wait until the server
+ # releases the exclusive byte-range lock it holds on locks\server.lock.
+ # The explicit Lock(0, 1) probe is required: a byte-range lock does not
+ # prevent opening the file, so a bare Open() would always succeed.
+ function Wait-ServerStopped($lockFile) {
+ $lockFile = Join-Path $PWD $lockFile
+ for ($i = 0; $i -lt 30; $i++) {
+ if (-not (Test-Path $lockFile)) { return }
+ try {
+ $fs = [System.IO.File]::Open($lockFile, 'Open', 'ReadWrite', 'ReadWrite')
+ try { $fs.Lock(0, 1); $fs.Unlock(0, 1); return } finally { $fs.Close() }
+ } catch { Start-Sleep -Seconds 2 }
+ }
+ throw "The server still holds the lock on ${lockFile}: the stop did not take effect"
+ }
set OPENDJ_JAVA_ARGS="-server -Xmx512m"
opendj-server-legacy\target\package\opendj\setup.bat -h localhost -p 1389 --ldapsPort 1636 --adminConnectorPort 4444 --enableStartTLS --generateSelfSignedCertificate --rootUserDN "cn=Directory Manager" --rootUserPassword password --baseDN dc=example,dc=com --sampleData 5000 --cli --acceptLicense --no-prompt
opendj-server-legacy\target\package\opendj\bat\status.bat --hostname localhost --bindDN "cn=Directory Manager" --bindPassword password --trustAll
@@ -325,6 +340,8 @@
opendj-server-legacy\target\package\opendj\bat\dsconfig.bat create-backend --hostname localhost --port 4444 --bindDN "cn=Directory Manager" --bindPassword password --backend-name=example2 --type je --set=base-dn:dc=example2,dc=com --set=enabled:true --no-prompt --trustAll
opendj-server-legacy\target\package\opendj\bat\makeldif.bat -o test.ldif -c suffix=dc=example2,dc=com opendj-server-legacy\target\package\opendj\config\MakeLDIF\example.template
opendj-server-legacy\target\package\opendj\bat\stop-ds.bat
+ if ($LASTEXITCODE -ne 0) { throw "stop-ds.bat failed with exit code $LASTEXITCODE" }
+ Wait-ServerStopped 'opendj-server-legacy\target\package\opendj\locks\server.lock'
echo "4.9.9.0" > opendj-server-legacy\target\package\opendj\config\buildinfo
opendj-server-legacy\target\package\opendj\upgrade.bat
opendj-server-legacy\target\package\opendj\bat\import-ldif.bat --offline --ldifFile test.ldif --backendID=example2
@@ -333,6 +350,8 @@
opendj-server-legacy\target\package\opendj\bat\rebuild-index.bat --bindDN "cn=Directory Manager" --bindPassword password --baseDN "dc=example2,dc=com" --rebuildAll --trustAll
opendj-server-legacy\target\package\opendj\bat\ldapsearch.bat --hostname localhost --port 1636 --bindDN "cn=Directory Manager" --bindPassword password --useSsl --trustAll --baseDN "dc=example2,dc=com" --searchScope sub "(uid=user.*)" dn | find /c '"dn:"' | findstr "10000"
opendj-server-legacy\target\package\opendj\bat\stop-ds.bat
+ if ($LASTEXITCODE -ne 0) { throw "stop-ds.bat failed with exit code $LASTEXITCODE" }
+ Wait-ServerStopped 'opendj-server-legacy\target\package\opendj\locks\server.lock'
opendj-server-legacy\target\package\opendj\bat\windows-service.bat --enableService
net start "OpenDJ Server"
if ($LASTEXITCODE -ne 0) { throw "net start 'OpenDJ Server' failed with exit code $LASTEXITCODE" }
diff --git a/opendj-server-legacy/resource/bin/stop-ds.bat b/opendj-server-legacy/resource/bin/stop-ds.bat
index 581f020..8848999 100644
--- a/opendj-server-legacy/resource/bin/stop-ds.bat
+++ b/opendj-server-legacy/resource/bin/stop-ds.bat
@@ -14,6 +14,7 @@
rem
rem Copyright 2006-2010 Sun Microsystems, Inc.
rem Portions Copyright 2011-2014 ForgeRock AS.
+rem Portions Copyright 2026 3A Systems, LLC.
setlocal
@@ -93,12 +94,19 @@
:stopUsingSystemCall
echo %SCRIPT%: stop using system call >> %LOG%
"%INSTALL_ROOT%\lib\winlauncher.exe" stop "%INSTANCE_ROOT%"
+if not %errorlevel% == 0 (
+ echo %SCRIPT%: winlauncher stop failed with error %errorlevel% >> %LOG%
+ exit /B %errorlevel%
+)
goto end
:restartUsingSystemCall
echo %SCRIPT%: restart using system call >> %LOG%
"%INSTALL_ROOT%\lib\winlauncher.exe" stop "%INSTANCE_ROOT%"
-if not %errorlevel% == 0 goto end
+if not %errorlevel% == 0 (
+ echo %SCRIPT%: winlauncher stop failed with error %errorlevel% >> %LOG%
+ exit /B %errorlevel%
+)
goto startUsingSystemCall
:stopUsingProtocol
diff --git a/opendj-server-legacy/src/build-tools/windows/winlauncher.c b/opendj-server-legacy/src/build-tools/windows/winlauncher.c
index ca301b6..31389d3 100644
--- a/opendj-server-legacy/src/build-tools/windows/winlauncher.c
+++ b/opendj-server-legacy/src/build-tools/windows/winlauncher.c
@@ -122,21 +122,26 @@
char pidFile[PATH_SIZE];
FILE *f;
char buf[BUF_SIZE];
- int read;
+ size_t nRead;
debug("Attempting to get the PID for the server rooted at '%s'.", instanceDir);
if (getPidFile(instanceDir, pidFile, PATH_SIZE))
{
if ((f = fopen(pidFile, "r")) != NULL)
{
- read = fread(buf, 1, sizeof(buf),f);
- debug("Read '%s' from the PID file '%s'.", buf, pidFile);
- }
-
- if (f != NULL)
- {
+ nRead = fread(buf, 1, sizeof(buf) - 1, f);
fclose(f);
- returnValue = (int)strtol(buf, (char **)NULL, 10);
+ buf[nRead] = '\0';
+ if (nRead > 0)
+ {
+ debug("Read '%s' from the PID file '%s'.", buf, pidFile);
+ returnValue = (int)strtol(buf, (char **)NULL, 10);
+ }
+ else
+ {
+ debugError("The PID file '%s' is empty.", pidFile);
+ returnValue = 0;
+ }
}
else
{
@@ -177,9 +182,20 @@
if (procHandle == NULL)
{
- debug("The process with pid=%d has already terminated.", pid);
- // process already dead
- processDead = TRUE;
+ DWORD lastError = GetLastError();
+ if (lastError == ERROR_INVALID_PARAMETER)
+ {
+ // no process with this pid exists: it has already terminated
+ debug("The process with pid=%d has already terminated.", pid);
+ processDead = TRUE;
+ }
+ else
+ {
+ // Access denied or any other error: the process may well still be
+ // running, so it must not be reported as stopped.
+ debugError("Failed to open the process (pid=%d) lastError=%d.", pid, lastError);
+ processDead = FALSE;
+ }
}
else
{
@@ -246,7 +262,7 @@
// immediately.
if ((fopen_s(&f, pidFile, "w") == 0) && (f != NULL))
{
- fprintf(f, "%d", pid);
+ fprintf(f, "%d\n", pid);
fclose (f);
returnValue = TRUE;
debug("Successfully put pid=%d in the pid file '%s'.", pid, pidFile);
@@ -427,6 +443,74 @@
// ----------------------------------------------------
+// Waits until the exclusive lock that the server holds on
+// locks\server.lock has been released, which is the definitive sign
+// that the server process is gone (the pid in the pid file may be
+// stale and belong to another process, so the outcome of killProcess
+// alone is not proof that the server was stopped).
+// Only a lock conflict (EACCES) is proof that the server still runs:
+// like isServerRunning in service.c, a lock file that cannot be opened
+// or an unexpected locking error is not treated as a running server.
+// Returns TRUE if the server no longer holds the lock (or holding it
+// cannot be proven) and FALSE if the lock conflict persists after the
+// bounded retries.
+// ----------------------------------------------------
+BOOL waitForLockRelease(const char* instanceDir)
+{
+ char lockFile[PATH_SIZE];
+ char* relativePath = "\\locks\\server.lock";
+ int nTries = 30;
+
+ if (!isSafePath(instanceDir)
+ || (strlen(relativePath) + strlen(instanceDir)) >= PATH_SIZE)
+ {
+ debugError("Unable to get the lock file name for instanceDir='%s'.", instanceDir);
+ return FALSE;
+ }
+ _snprintf(lockFile, PATH_SIZE, "%s%s", instanceDir, relativePath);
+
+ while (nTries > 0)
+ {
+ int fd;
+ int lockingError;
+ if (!fileExists(lockFile))
+ {
+ debug("Lock file '%s' does not exist, so the server is stopped.", lockFile);
+ return TRUE;
+ }
+ fd = _open(lockFile, _O_RDWR);
+ if (fd == -1)
+ {
+ debug("Could not open the lock file '%s' (errno=%d), so the server is considered stopped.",
+ lockFile, errno);
+ return TRUE;
+ }
+ if (_locking(fd, LK_NBLCK, 1) != -1)
+ {
+ _locking(fd, LK_UNLCK, 1);
+ _close(fd);
+ debug("Acquired the lock on '%s', so the server is stopped.", lockFile);
+ return TRUE;
+ }
+ lockingError = errno;
+ _close(fd);
+ if (lockingError != EACCES)
+ {
+ debugError("Unexpected error locking '%s': %d", lockFile, lockingError);
+ return TRUE;
+ }
+ nTries--;
+ debug("The server still holds the lock on '%s'. Sleeping for 1 second and will try %d more time(s).",
+ lockFile, nTries);
+ Sleep(1000);
+ }
+
+ debugError("The server did not release the lock on '%s'.", lockFile);
+ return FALSE;
+} // waitForLockRelease
+
+
+// ----------------------------------------------------
// Function called when we want to stop the server.
// This code is called by the stop-ds.bat batch file to stop the server
// in windows.
@@ -443,13 +527,14 @@
// sets the pid file to be deleted on the exit of the process
// the file is not always deleted.
//
-// Returns 0 if the instance could be stopped using the
-// pid stored in a file of the server installation and
-// -1 otherwise.
+// Returns 0 if the server no longer holds the server lock (the process
+// found in the pid file, if any, was killed and the lock was released)
+// and -1 otherwise.
// ----------------------------------------------------
int stop(const char* instanceDir)
{
int returnCode = -1;
+ BOOL mayHaveStopped = FALSE;
int childPid;
@@ -459,15 +544,33 @@
if (childPid != 0)
{
- if (killProcess(childPid))
- {
- returnCode = 0;
- deletePidFile(instanceDir);
- }
+ mayHaveStopped = killProcess(childPid);
}
else
{
- debug("Could not stop the server running at root '%s' because the pid could not be located.", instanceDir);
+ // The pid file is typically missing because the server has already
+ // stopped (or is stopping right now): the lock probe below gives the
+ // definitive answer.
+ debug("Could not locate the pid of the server running at root '%s': relying on the server lock alone.", instanceDir);
+ mayHaveStopped = TRUE;
+ }
+
+ if (mayHaveStopped)
+ {
+ if (waitForLockRelease(instanceDir))
+ {
+ returnCode = 0;
+ if (childPid != 0)
+ {
+ deletePidFile(instanceDir);
+ }
+ }
+ else
+ {
+ char * msg = "The server at '%s' is still running: it did not release the server lock.\n";
+ debugError(msg, instanceDir);
+ fprintf(stderr, msg, instanceDir);
+ }
}
return returnCode;
diff --git a/opendj-server-legacy/src/build-tools/windows/winlauncher.h b/opendj-server-legacy/src/build-tools/windows/winlauncher.h
index 0a02fda..865a6c9 100644
--- a/opendj-server-legacy/src/build-tools/windows/winlauncher.h
+++ b/opendj-server-legacy/src/build-tools/windows/winlauncher.h
@@ -12,11 +12,16 @@
* information: "Portions Copyright [year] [name of copyright owner]".
*
* Copyright 2008 Sun Microsystems, Inc.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
#include "common.h"
+#include <errno.h>
+#include <fcntl.h>
+#include <io.h>
#include <stdio.h>
#include <stdlib.h>
+#include <sys/locking.h>
#include <sys/stat.h>
#include <process.h>
--
Gitblit v1.10.0