Fix stop-edge.sh not stopping its own process when IOTDB_HOME is set

.github/scripts/test-edge-ops.py

@@ -24,8 +24,11 @@
 import os
 from pathlib import Path
 import shutil
+import signal
 import subprocess
+import sys
 import tempfile
+import time
 import unittest
 import xml.etree.ElementTree as ET
 
@@ -411,6 +414,234 @@ def test_windows_force_stop_returns_without_pausing(self):
         self.assertNotIn("Press any key", result.stdout)
 
 
+@unittest.skipIf(WINDOWS, "Unix stop-edge.sh")
+class EdgeStopTest(unittest.TestCase):
+    """Run the real stop-edge.sh against stand-in Edge processes that only sleep.
+
+    start-edge.sh needs a JVM, so each stand-in carries the -DIOTDB_HOME that
+    start-edge.sh would pass to it: the value initEdgeHome derives for it.
+    """
+
+    def setUp(self):
+        temporary = tempfile.TemporaryDirectory(prefix="iotdb-edge-stop-")
+        self.addCleanup(temporary.cleanup)
+        self.root = Path(temporary.name).resolve()
+        # Identifies this test's stand-ins, so that cleanup never touches others.
+        self.marker = "-Dedge.stop.test=" + self.root.as_posix()
+        self.env = os.environ.copy()
+        self.env.pop("IOTDB_HOME", None)
+        self.env.pop("CDPATH", None)
+
+    def install(self, relative):
+        home = self.root / relative
+        for script in (
+            "sbin/start-edge.sh",
+            "sbin/stop-edge.sh",
+            "conf/iotdb-common.sh",
+        ):
+            (home / script).parent.mkdir(parents=True, exist_ok=True)
+            shutil.copyfile(REPOSITORY / "scripts" / script, home / script)
+        return home
+
+    def link(self, relative, target):
+        path = self.root / relative
+        if path.is_symlink():
+            path.unlink()
+        path.symlink_to(target, target_is_directory=True)
+
+    def symlink_layout(self):
+        # x/link/../edge is y/edge physically, while x/edge is another installation.
+        (self.root / "y/sub").mkdir(parents=True)
+        self.install("y/edge")
+        self.install("x/edge")
+        self.link("x/link", "../y/sub")
+
+    def bash(self, arguments, iotdb_home=None, cwd=None, **variables):
+        env = dict(self.env, **variables)
+        if iotdb_home is not None:
+            env["IOTDB_HOME"] = iotdb_home
+        return subprocess.run(
+            ["bash", *arguments],
+            env=env,
+            cwd=cwd or self.root,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            text=True,
+            errors="replace",
+            timeout=60,
+        )
+
+    def started_home(self, script, iotdb_home=None, **variables):
+        """The -DIOTDB_HOME that start-edge.sh at this path passes to the JVM."""
+        result = self.bash(
+            (
+                "-c",
+                'source "$(dirname "$1")/../conf/iotdb-common.sh"'
+                ' && initEdgeHome "$1" && printf "%s" "$IOTDB_HOME"',
+                "bash",
+                script,
+            ),
+            iotdb_home,
+            **variables,
+        )
+        self.assertEqual(result.returncode, 0, result.stderr)
+        return result.stdout
+
+    def stop(self, script, iotdb_home=None, cwd=None, **variables):
+        result = self.bash((script,), iotdb_home, cwd, **variables)
+        self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+        return result
+
+    def spawn(self, command, cwd=None):
+        # Detached through a short-lived parent, like the nohup'ed JVM, so that
+        # the process is reaped once it exits: a zombie child of this test would
+        # still look alive to stop-edge.sh.
+        launcher = subprocess.run(
+            [
+                sys.executable,
+                "-c",
+                "import subprocess, sys; print(subprocess.Popen(sys.argv[1:],"
+                " stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL,"
+                " stderr=subprocess.DEVNULL).pid)",
+                *command,
+            ],
+            cwd=cwd or self.root,
+            stdout=subprocess.PIPE,
+            text=True,
+            check=True,
+            timeout=30,
+        )
+        pid = int(launcher.stdout)
+        self.addCleanup(self.kill, pid)
+        return pid
+
+    def start(self, home, cwd=None):
+        """Start a stand-in for the JVM and write edge.pid where start-edge.sh does."""
+        cwd = cwd or self.root
+        pid = self.spawn(
+            [
+                sys.executable,
+                "-c",
+                "import time; time.sleep(600)",
+                self.marker,
+                "-DIOTDB_HOME=" + home,
+                "-DCONFIGNODE_HOME=" + home,
+                "org.apache.iotdb.edge.EdgeNode",
+                "-s",
+            ],
+            cwd,
+        )
+        (Path(cwd) / home / "edge.pid").write_text("{}\n".format(pid))
+        return pid
+
+    def command(self, pid):
+        return subprocess.run(
+            ["ps", "-ww", "-p", str(pid), "-o", "command="],
+            stdout=subprocess.PIPE,
+            text=True,
+            errors="replace",
+        ).stdout
+
+    def running(self, pid):
+        return self.marker in self.command(pid)
+
+    def kill(self, pid):
+        if self.running(pid):
+            os.kill(pid, signal.SIGKILL)
+
+    def test_symlinked_home_is_stopped_through_the_real_path(self):
+        # apache/iotdb#18545
+        self.install("edge")
+        self.link("link", "edge")
+        home = self.started_home("edge/sbin/start-edge.sh", str(self.root / "link"))
+        pid = self.start(home)
+        self.stop("edge/sbin/stop-edge.sh")
+        self.assertFalse(self.running(pid))
+
+    def test_stop_after_the_symlink_is_re_pointed(self):
+        self.install("edge-v1")
+        self.install("edge-v2")
+        for iotdb_home in (None, str(self.root / "iotdb")):
+            with self.subTest(IOTDB_HOME=iotdb_home):
+                self.link("iotdb", "edge-v1")
+                pid = self.start(
+                    self.started_home("iotdb/sbin/start-edge.sh", iotdb_home)
+                )
+                self.link("iotdb", "edge-v2")
+                self.stop("iotdb/sbin/stop-edge.sh", iotdb_home)
+                self.assertFalse(self.running(pid))
+
+    def test_relative_home_of_another_installation_is_left_alone(self):
+        # Recorded as-is by the start-edge.sh of earlier releases.
+        pid = self.start(".", cwd=self.install("edge-a"))
+        self.install("edge-b")
+        for iotdb_home in (None, "."):
+            with self.subTest(IOTDB_HOME=iotdb_home):
+                self.stop("sbin/stop-edge.sh", iotdb_home, cwd=self.root / "edge-b")
+                self.assertTrue(self.running(pid))
+
+    def test_symlink_followed_by_dot_dot_in_the_home(self):
+        self.symlink_layout()
+        given = str(self.root / "x/link/../edge")
+        home = self.started_home("y/edge/sbin/start-edge.sh", given)
+        self.assertEqual(home, given)
+        pid = self.start(home)
+        self.stop("x/edge/sbin/stop-edge.sh")
+        self.assertTrue(self.running(pid))
+        self.stop("y/edge/sbin/stop-edge.sh")
+        self.assertFalse(self.running(pid))
+
+    def test_symlink_followed_by_dot_dot_in_the_script_path(self):
+        self.symlink_layout()
+        pid = self.start(self.started_home("x/link/../edge/sbin/start-edge.sh"))
+        self.stop("x/link/../edge/sbin/stop-edge.sh")
+        self.assertFalse(self.running(pid))
+
+    def test_process_without_iotdb_home_is_not_matched(self):
+        home = self.install("edge")
+        # Everything before the first " -D" names the installation, and edge.pid
+        # points at the process.
+        pid = self.spawn(
+            [
+                "perl",
+                "-e",
+                "$0 = shift; sleep 600",
+                "{} -Dfoo=1 {} org.apache.iotdb.edge.EdgeNode -s".format(
+                    home, self.marker
+                ),
+            ]
+        )
+        for _ in range(100):
+            if self.command(pid).startswith(str(home)):
+                break
+            time.sleep(0.05)
+        (home / "edge.pid").write_text("{}\n".format(pid))
+        self.stop("edge/sbin/stop-edge.sh")
+        self.assertTrue(self.running(pid))
+
+    def test_cdpath_is_not_used(self):
+        self.install("edge")
+        (self.root / "elsewhere/edge/sbin").mkdir(parents=True)
+        cdpath = {"CDPATH": str(self.root / "elsewhere")}
+        home = self.started_home("edge/sbin/start-edge.sh", **cdpath)
+        self.assertEqual(home, str(self.root / "edge"))
+        pid = self.start(home)
+        self.stop("edge/sbin/stop-edge.sh", **cdpath)
+        self.assertFalse(self.running(pid))
+
+    def test_home_of_another_installation_is_reported(self):
+        self.install("edge-a")
+        own = self.install("edge-b")
+        result = self.stop("edge-b/sbin/stop-edge.sh", str(self.root / "edge-a"))
+        self.assertIn("belongs to {}".format(own), result.stderr)
+
+    def test_missing_home_is_an_error(self):
+        self.install("edge")
+        result = self.bash(("edge/sbin/stop-edge.sh",), str(self.root / "missing"))
+        self.assertNotEqual(result.returncode, 0, result.stdout)
+        self.assertIn("not an accessible directory", result.stderr)
+
+
 class AssemblyTest(unittest.TestCase):
     @staticmethod
     def packaged(descriptor, path):

AGENTS.md

@@ -83,7 +83,7 @@ Keep IT-specific rules and commands in that module-level file.
 - `EdgeNode` (`iotdb-core/confignode/src/main/java/org/apache/iotdb/edge/EdgeNode.java`) starts the ConfigNode and DataNode in one JVM for resource-constrained single-node deployments.
 - `distribution/src/assembly/edge.xml` produces the additional `apache-iotdb-<version>-edge-bin.zip` artifact. Keep its `edge-bin` assembly id unique so it cannot replace an existing attached artifact.
 - Edge-specific system defaults live under `iotdb-core/node-commons/src/assembly/resources/conf/edge/`. Standard node, all-in-one, and integration-test assemblies must continue to exclude `edge/**`.
-- Edge launchers must reuse the configuration-aware port checks in `scripts/conf/iotdb-common.sh`. Stop scripts must match both the `EdgeNode` main class and the exact `IOTDB_HOME`; never fall back to a global process-name kill.
+- Edge launchers must reuse the configuration-aware port checks in `scripts/conf/iotdb-common.sh`. Start and stop scripts must derive `IOTDB_HOME` with the same shared function (`initEdgeHome`). Stop scripts must match both the `EdgeNode` main class and the `IOTDB_HOME` recorded at start, either exactly or, for an absolute recorded path, after resolving symlinks on both sides; never fall back to a global process-name kill.
 - Package node-independent tools that work with the combined process. Exclude scripts that require standalone node launchers or environment files, including destructive, daemon-management, and health-check scripts.
 - After packaging changes, run a clean distribution build, check that every existing distribution artifact is still produced, and inspect the Edge zip contents and startup/shutdown behavior.
 

integration-test/src/test/java/org/apache/iotdb/edge/it/IoTDBEdgeBasicIT.java

@@ -133,6 +133,7 @@ public static void tearDown() throws Exception {
       if (edgeHome != null && Files.isRegularFile(edgeHome.resolve("sbin/stop-edge.sh"))) {
         try {
           runScript(edgeHome.resolve("sbin/stop-edge.sh"), STOP_SCRIPT_LOG);
+          assertEdgeProcessExited();
         } catch (Exception | AssertionError e) {
           stopFailure = new AssertionError("Failed to stop IoTDB Edge with stop-edge.sh", e);
         }
@@ -541,6 +542,21 @@ private static void runScript(final Path script, final Path outputFile) throws E
     }
   }
 
+  private static void assertEdgeProcessExited() throws InterruptedException {
+    // stop-edge.sh also exits 0 when it finds no matching process, so check the process itself.
+    // The short wait covers its "kill -9" fallback, which does not wait for the exit.
+    if (edgePid <= 0) {
+      return;
+    }
+    final ProcessHandle process = ProcessHandle.of(edgePid).orElse(null);
+    for (int i = 0; i < 10 && process != null && process.isAlive(); i++) {
+      Thread.sleep(500);
+    }
+    assertFalse(
+        "IoTDB Edge process " + edgePid + " is still running after stop-edge.sh returned",
+        process != null && process.isAlive());
+  }
+
   private static void stopProcessForciblyIfNeeded() throws InterruptedException {
     if (edgePid <= 0) {
       return;

scripts/conf/iotdb-common.sh

@@ -508,3 +508,42 @@ calculate_memory_sizes()
     ON_HEAP_MEMORY="${on_heap_memory_size_in_mb}M"
     OFF_HEAP_MEMORY="${off_heap_memory_size_in_mb}M"
 }
+
+# Prints the physical absolute path of directory $1, or nothing if it cannot be
+# entered. "cd -P" is required: a plain "cd" collapses ".." logically, which
+# would resolve "<symlink>/../x" against the symlink's parent instead of its
+# target. CDPATH is cleared so that a relative name is not looked up in it.
+resolvePhysicalDir()
+{
+  [ -n "$1" ] || return 1
+  (CDPATH= cd -P -- "$1" 2>/dev/null && pwd -P)
+}
+
+# Sets IOTDB_HOME for start-edge.sh and stop-edge.sh, which both call this with
+# their own "$0". start-edge.sh hands the value to the JVM as -DIOTDB_HOME and
+# stop-edge.sh finds the process by it, so the two must derive it the same way.
+# The value is only made absolute; symlinks are deliberately not resolved, so
+# that an installation started through a symlink keeps that identity after the
+# symlink is re-pointed to another version.
+initEdgeHome()
+{
+  if [ -z "${IOTDB_HOME}" ]; then
+    IOTDB_HOME="$(CDPATH= cd -- "$(dirname "$1")"/.. && pwd)" || return 1
+  fi
+  case "${IOTDB_HOME}" in
+    /*) ;;
+    *) IOTDB_HOME="$(pwd)/${IOTDB_HOME}" ;;
+  esac
+  local physical_home
+  local script_home
+  physical_home="$(resolvePhysicalDir "${IOTDB_HOME}")"
+  if [ -z "${physical_home}" ]; then
+    echo "IOTDB_HOME ${IOTDB_HOME} is not an accessible directory." >&2
+    return 1
+  fi
+  script_home="$(resolvePhysicalDir "$(dirname "$1")/..")"
+  if [ "${physical_home}" != "${script_home}" ]; then
+    echo "Warning: IOTDB_HOME is ${IOTDB_HOME}, but $(basename "$1") belongs to ${script_home}." >&2
+  fi
+  export IOTDB_HOME
+}

scripts/sbin/start-edge.sh

@@ -20,17 +20,16 @@
 
 # Start IoTDB Edge: ConfigNode + DataNode in one JVM process.
 
-if [ -z "${IOTDB_HOME}" ]; then
-    export IOTDB_HOME="$(cd "$(dirname "$0")"/.. && pwd)"
-fi
+source "$(dirname "$0")/../conf/iotdb-common.sh"
+# stop-edge.sh derives IOTDB_HOME with the same function and matches on it.
+initEdgeHome "$0" || exit 1
 if [ -z "${IOTDB_CONF}" ]; then
     export IOTDB_CONF=${IOTDB_HOME}/conf
 fi
 export IOTDB_DATA_HOME=${IOTDB_DATA_HOME:-${IOTDB_HOME}}
 export IOTDB_LOG_DIR=${IOTDB_LOG_DIR:-${IOTDB_HOME}/logs}
 mkdir -p "${IOTDB_LOG_DIR}"
 
-source "$(dirname "$0")/../conf/iotdb-common.sh"
 export CONFIGNODE_HOME=${IOTDB_HOME}
 export CONFIGNODE_DATA_HOME=${IOTDB_DATA_HOME}
 export CONFIGNODE_CONF=${IOTDB_CONF}

scripts/sbin/stop-edge.sh

@@ -20,7 +20,13 @@
 
 # Stop IoTDB Edge (the merged ConfigNode + DataNode process).
 
-IOTDB_HOME="$(cd "$(dirname "$0")"/.. && pwd)"
+source "$(dirname "$0")/../conf/iotdb-common.sh"
+# Same derivation as start-edge.sh, which records the result as -DIOTDB_HOME.
+initEdgeHome "$0" || exit 1
+
+# The physical path, so that the same installation reached through a different
+# path still compares equal in the fallback below.
+IOTDB_HOME_RESOLVED="$(resolvePhysicalDir "${IOTDB_HOME}")"
 
 PID_FILE="${IOTDB_HOME}/edge.pid"
 
@@ -30,10 +36,28 @@ is_same_edge_home() {
         *"-DIOTDB_HOME=${IOTDB_HOME} "*|*"-DIOTDB_HOME=${IOTDB_HOME}")
             return 0
             ;;
+        *-DIOTDB_HOME=*)
+            ;;
         *)
             return 1
             ;;
     esac
+    # Fall back to comparing resolved paths, so that a process started with
+    # IOTDB_HOME pointing at a symlink is still recognised when stopped through
+    # the real path, and the other way round. The value is delimited by the next
+    # " -D", which start-edge.sh always emits after -DIOTDB_HOME. If a hand-built
+    # command line ends with -DIOTDB_HOME, the extraction keeps the trailing
+    # arguments, the resolution below fails and the process is not matched.
+    local home="${command_line#*-DIOTDB_HOME=}"
+    home="${home%% -D*}"
+    # Only absolute values can be resolved from here. A relative one such as "."
+    # is meaningful in the started process's working directory, not in ours, so
+    # resolving it here could match an unrelated installation.
+    case "$home" in
+        /*) ;;
+        *) return 1 ;;
+    esac
+    [ "$(resolvePhysicalDir "$home")" = "${IOTDB_HOME_RESOLVED}" ]
 }
 
 is_edge_process() {