Skip to content

fix: use CREATE_NO_WINDOW so Windows hook miner spawns don't flash a console (#1783) #1848

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
eldar702 wants to merge 1 commit into
base: develop
Choose a base branch
from
+22 −10
Open

fix: use CREATE_NO_WINDOW so Windows hook miner spawns don't flash a console (#1783) #1848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions mempalace/hooks_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@


def _detached_popen_kwargs() -> dict:
"""Kwargs that fully detach a Popen child so the hook process can exit.
"""Kwargs that give a Popen child a hidden console so the hook can exit.

Without these, Windows holds the parent open until the child closes the
inherited stdout/stderr handles — manifesting as "Stop hook hangs" at
Expand All @@ -36,7 +36,7 @@ def _detached_popen_kwargs() -> dict:
kwargs: dict = {"stdin": subprocess.DEVNULL, "close_fds": True}
if os.name == "nt":
flags = 0
for name in ("DETACHED_PROCESS", "CREATE_NEW_PROCESS_GROUP", "CREATE_BREAKAWAY_FROM_JOB"):
for name in ("CREATE_NO_WINDOW", "CREATE_NEW_PROCESS_GROUP", "CREATE_BREAKAWAY_FROM_JOB"):

@gemini-code-assist gemini-code-assist Bot Jun 21, 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.

medium

This change replaces DETACHED_PROCESS with CREATE_NO_WINDOW to prevent console flashing on Windows. However, the exact same pattern of using DETACHED_PROCESS is present in mempalace/daemon.py inside _detached_kwargs (line 978):

        for name in ("DETACHED_PROCESS", "CREATE_NEW_PROCESS_GROUP", "CREATE_BREAKAWAY_FROM_JOB"):

According to the repository's general rules, when addressing a common issue or pattern that is present in multiple sibling implementations across the codebase, we should avoid applying a one-off fix to a single instance. Instead, we should maintain repository-wide consistency by deferring the fix to a dedicated change that addresses all occurrences together.

References
  1. When addressing a common issue or pattern (such as unsafe SQLite URI path encoding) that is present in multiple sibling implementations across the codebase, avoid applying a one-off fix to a single instance. Instead, maintain repository-wide consistency by deferring the fix to a dedicated change that addresses all occurrences together.

@eldar702 eldar702 Jun 21, 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.

Thanks for flagging this. I looked at daemon.py's _detached_kwargs, and I think the two cases are genuinely different rather than one shared pattern:

So the differing flag is intentional and load-bearing, not a one-off instance of a common pattern. I'd rather keep this PR scoped to the hook-spawn path; happy to add a short comment on the daemon side documenting why it keeps DETACHED_PROCESS if that'd help future readers.

flags |= getattr(subprocess, name, 0)
if flags:
kwargs["creationflags"] = flags
Expand Down
28 changes: 20 additions & 8 deletions tests/test_hooks_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1035,17 +1035,26 @@ def test_detached_popen_kwargs_posix(monkeypatch):


def test_detached_popen_kwargs_windows(monkeypatch):
"""On Windows, kwargs include creationflags that fully detach the child.

Without these, the parent hook hangs at session end on Windows because
the child's inherited stdout/stderr handles keep the parent's exit
blocked (#1268 root cause for the Python hook path).
"""On Windows, the miner child gets a hidden console (CREATE_NO_WINDOW),
not a detached/no-console child (DETACHED_PROCESS).

DETACHED_PROCESS gave the child no console at all, which caused any
console grandchild it spawned to allocate a fresh *visible* window
(#1783). CREATE_NO_WINDOW gives a real-but-invisible console that all
descendants inherit, so nothing flashes — while still fixing the
#1268 hang (stdin=DEVNULL, close_fds, explicit stdout/stderr redirect,
and CREATE_NEW_PROCESS_GROUP for the signal boundary are unchanged).
Per the Win32 CreateProcess docs CREATE_NO_WINDOW is ignored when OR'd
with DETACHED_PROCESS, so the two must be mutually exclusive.
"""
from mempalace.hooks_cli import _detached_popen_kwargs

monkeypatch.setattr("mempalace.hooks_cli.os.name", "nt")
# Simulate Windows-only Popen flag constants. Patch on the imported
# subprocess module within hooks_cli so getattr() picks them up.
# Simulate Windows-only Popen flag constants on the imported subprocess
# module so getattr() picks them up cross-platform.
monkeypatch.setattr(
"mempalace.hooks_cli.subprocess.CREATE_NO_WINDOW", 0x08000000, raising=False
)
monkeypatch.setattr(
"mempalace.hooks_cli.subprocess.DETACHED_PROCESS", 0x00000008, raising=False
)
Expand All @@ -1056,7 +1065,10 @@ def test_detached_popen_kwargs_windows(monkeypatch):
assert kwargs.get("stdin") is subprocess.DEVNULL
assert kwargs.get("close_fds") is True
flags = kwargs.get("creationflags", 0)
assert flags & 0x00000008, "DETACHED_PROCESS must be set"
assert flags & 0x08000000, "CREATE_NO_WINDOW must be set"
assert not (flags & 0x00000008), (
"DETACHED_PROCESS must NOT be set (it suppresses CREATE_NO_WINDOW)"
)
assert flags & 0x00000200, "CREATE_NEW_PROCESS_GROUP must be set"


Expand Down
Loading