Skip to content

Commit ce67b5d

Browse files
fix: stop using a raw substring match to detect concore processes
concore stop / concore status flagged any process as a concore process just because "concore" appeared anywhere in its cmdline. That matches anything run from a directory that happens to be named "concore" (the default clone directory name for this repo), which has nothing to do with a real concore node and would get force killed on the next concore stop. Match is now based on the generated concorekill.bat filename, or on the process's actual working directory containing the runtime marker files mkconcore.py writes into every generated study (concore.iport plus concore.py/concoredocker.py), instead of a plain text search. Fixes #580
1 parent c20b909 commit ce67b5d

4 files changed

Lines changed: 124 additions & 24 deletions

File tree

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import os
2+
3+
4+
def has_concore_markers(cwd):
5+
"""True only if `cwd` looks like an actual concore-generated node
6+
working directory, i.e. it contains the runtime files mkconcore.py
7+
copies into every generated study (concore.iport/oport plus the
8+
runtime module itself)."""
9+
if not cwd:
10+
return False
11+
try:
12+
if not os.path.isfile(os.path.join(cwd, "concore.iport")):
13+
return False
14+
return os.path.isfile(os.path.join(cwd, "concore.py")) or os.path.isfile(
15+
os.path.join(cwd, "concoredocker.py")
16+
)
17+
except OSError:
18+
return False
19+
20+
21+
def is_concore_process(cmdline, cwd):
22+
"""Decide whether a process is an actual concore node process.
23+
24+
A plain substring check like "concore" in the joined cmdline used to
25+
be used here, which matches anything launched from a directory that
26+
merely happens to have "concore" in its path (the default clone
27+
directory name for this repo among other things) and has nothing to
28+
do with concore at all. Instead, only match the generated kill
29+
script by exact filename, or a process whose working directory
30+
actually contains the concore runtime marker files.
31+
"""
32+
if any(os.path.basename(str(item)).lower() == "concorekill.bat" for item in cmdline):
33+
return True
34+
return has_concore_markers(cwd)

‎concore_cli/commands/status.py‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
from rich.panel import Panel
55
from datetime import datetime
66

7+
from concore_cli.commands._process_match import is_concore_process
8+
79

810
def show_status(console):
911
console.print("[cyan]Scanning for concore processes...[/cyan]\n")
@@ -18,24 +20,18 @@ def show_status(console):
1820
):
1921
try:
2022
cmdline = proc.info.get("cmdline") or []
21-
name = proc.info.get("name", "").lower()
2223

2324
if proc.info["pid"] == current_pid:
2425
continue
2526

2627
cmdline_str = " ".join(cmdline) if cmdline else ""
2728

28-
is_concore = (
29-
"concore" in cmdline_str.lower()
30-
or "concore.py" in cmdline_str.lower()
31-
or any("concorekill.bat" in str(item) for item in cmdline)
32-
or (
33-
name in ["python.exe", "python", "python3"]
34-
and "concore" in cmdline_str
35-
)
36-
)
29+
try:
30+
cwd = proc.cwd()
31+
except (psutil.NoSuchProcess, psutil.AccessDenied, OSError):
32+
cwd = None
3733

38-
if is_concore:
34+
if is_concore_process(cmdline, cwd):
3935
try:
4036
create_time = datetime.fromtimestamp(proc.info["create_time"])
4137
uptime = datetime.now() - create_time

‎concore_cli/commands/stop.py‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
import sys
55
from rich.panel import Panel
66

7+
from concore_cli.commands._process_match import is_concore_process
8+
79

810
def stop_all(console):
911
console.print("[cyan]Finding concore processes...[/cyan]\n")
@@ -18,20 +20,13 @@ def stop_all(console):
1820
continue
1921

2022
cmdline = proc.info.get("cmdline") or []
21-
name = proc.info.get("name", "").lower()
22-
cmdline_str = " ".join(cmdline) if cmdline else ""
23-
24-
is_concore = (
25-
"concore" in cmdline_str.lower()
26-
or "concore.py" in cmdline_str.lower()
27-
or any("concorekill.bat" in str(item) for item in cmdline)
28-
or (
29-
name in ["python.exe", "python", "python3"]
30-
and "concore" in cmdline_str
31-
)
32-
)
3323

34-
if is_concore:
24+
try:
25+
cwd = proc.cwd()
26+
except (psutil.NoSuchProcess, psutil.AccessDenied, OSError):
27+
cwd = None
28+
29+
if is_concore_process(cmdline, cwd):
3530
processes_to_kill.append(proc)
3631
except (psutil.NoSuchProcess, psutil.AccessDenied):
3732
# Process already exited or access denied; continue

‎tests/test_process_match.py‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
"""Tests for concore_cli.commands._process_match (Issue #580).
2+
3+
`concore stop` / `concore status` used to flag any process as a
4+
"concore process" just because "concore" showed up somewhere in its
5+
cmdline, which is true for basically anything launched from inside a
6+
folder named "concore" (the default clone directory name for this
7+
repo) and has nothing to do with a real concore node.
8+
"""
9+
10+
import os
11+
12+
from concore_cli.commands._process_match import has_concore_markers, is_concore_process
13+
14+
15+
class TestHasConcoreMarkers:
16+
def test_true_when_iport_and_runtime_file_present(self, tmp_path):
17+
(tmp_path / "concore.iport").write_text("{}")
18+
(tmp_path / "concore.py").write_text("")
19+
assert has_concore_markers(str(tmp_path)) is True
20+
21+
def test_true_with_docker_runtime_file(self, tmp_path):
22+
(tmp_path / "concore.iport").write_text("{}")
23+
(tmp_path / "concoredocker.py").write_text("")
24+
assert has_concore_markers(str(tmp_path)) is True
25+
26+
def test_false_without_iport(self, tmp_path):
27+
(tmp_path / "concore.py").write_text("")
28+
assert has_concore_markers(str(tmp_path)) is False
29+
30+
def test_false_without_runtime_file(self, tmp_path):
31+
(tmp_path / "concore.iport").write_text("{}")
32+
assert has_concore_markers(str(tmp_path)) is False
33+
34+
def test_false_for_empty_cwd(self):
35+
assert has_concore_markers(None) is False
36+
assert has_concore_markers("") is False
37+
38+
def test_false_for_unrelated_folder_literally_named_concore(self, tmp_path):
39+
# A folder just happening to be named "concore" (e.g. a plain
40+
# git clone of this repo) is not, by itself, a running node's
41+
# working directory.
42+
concore_dir = tmp_path / "concore"
43+
concore_dir.mkdir()
44+
(concore_dir / "README.md").write_text("")
45+
assert has_concore_markers(str(concore_dir)) is False
46+
47+
48+
class TestIsConcoreProcess:
49+
def test_true_for_generated_kill_script(self):
50+
cmdline = [r"C:\studies\run1\concorekill.bat"]
51+
assert is_concore_process(cmdline, cwd=None) is True
52+
53+
def test_true_when_cwd_has_markers(self, tmp_path):
54+
(tmp_path / "concore.iport").write_text("{}")
55+
(tmp_path / "concore.py").write_text("")
56+
cmdline = ["python", "controller.py"]
57+
assert is_concore_process(cmdline, cwd=str(tmp_path)) is True
58+
59+
def test_false_for_unrelated_process_in_a_concore_named_folder(self, tmp_path):
60+
# This is the actual bug: previously, having "concore" anywhere
61+
# in the argv (e.g. a path under a folder named "concore") was
62+
# enough to be treated as a concore process and get killed.
63+
concore_dir = tmp_path / "concore"
64+
concore_dir.mkdir()
65+
cmdline = ["node", os.path.join(str(concore_dir), "tool.js")]
66+
assert is_concore_process(cmdline, cwd=str(concore_dir)) is False
67+
68+
def test_false_for_unrelated_python_script_mentioning_concore(self, tmp_path):
69+
concore_dir = tmp_path / "concore"
70+
concore_dir.mkdir()
71+
cmdline = ["python", os.path.join(str(concore_dir), "unrelated_report.py")]
72+
assert is_concore_process(cmdline, cwd=str(concore_dir)) is False
73+
74+
def test_false_for_empty_cmdline_and_cwd(self):
75+
assert is_concore_process([], None) is False

0 commit comments

Comments
 (0)