Skip to content

diagnose(shell): settle the owned desktop before reporting a leak (#160) #199

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

Merged
JavaZeroo merged 1 commit into from
Oct 29, 2026
Merged

diagnose(shell): settle the owned desktop before reporting a leak (#160) #199

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
24 changes: 20 additions & 4 deletions experiments/shell/run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,12 +402,28 @@ def __init__(self):
if not self.handle:
raise ctypes.WinError(ctypes.get_last_error())

def close(self):
def close(self, record=None):
if self.handle:
if not self.api.CloseDesktop(self.handle):
raise ctypes.WinError(ctypes.get_last_error())
self.handle = None
return desktop_gone(self.name)
gone = desktop_gone(self.name)
if gone or record is None:
return gone
# A desktop handle outlives its owner only while some child still holds
# one; settle briefly and keep the attempts so a failure shows whether the
# desktop was still held or never closed. Nothing is added on success.
attempts = [{"elapsed_ms": 0, "gone": False}]
began = time.monotonic()

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Import time before entering the desktop-settle path

When the first desktop_gone() check returns false, this call raises NameError: name 'time' is not defined because the module never imports time. This prevents all retries and leaves desktop_close_settle unwritten—the exact transient-desktop scenario this change is intended to diagnose still fails, now without the intended evidence. In the cleanup-only call site, the exception also bypasses the except OSError handler and can prevent sample.json from being saved. This is reproducible by mocking desktop_gone to return false and calling close({}).

Useful? React with 👍 / 👎.

for _ in range(10):
time.sleep(0.05)
gone = desktop_gone(self.name)
attempts.append({"elapsed_ms": int((time.monotonic() - began) * 1000), "gone": gone})
if gone:
break
if not gone:
record["desktop_close_settle"] = attempts
return gone


def execute(exe, mode, out, timeout=40, settings_source=None, *, resources=False):
Expand Down Expand Up @@ -460,7 +476,7 @@ def execute(exe, mode, out, timeout=40, settings_source=None, *, resources=False
stderr=stderr.decode("utf-8", "replace"), job_after=job.drain())
if record["job_after"]["active"]:
raise ValueError("owned processes remained")
record["desktop_gone"] = desktop.close()
record["desktop_gone"] = desktop.close(record)
if not record["desktop_gone"]:
raise ValueError("owned desktop remained")
if mode != "watchdog" and (record["timed_out"] or record["exit_code"]):
Expand Down Expand Up @@ -566,7 +582,7 @@ def execute(exe, mode, out, timeout=40, settings_source=None, *, resources=False
job.close()
if desktop is not None and desktop.handle:
try:
record["desktop_gone"] = desktop.close()
record["desktop_gone"] = desktop.close(record)
if not record["desktop_gone"]:
cleanup_errors.append("owned desktop remained after cleanup")
except OSError as error:
Expand Down
Loading