Skip to content
Closed
Changes from 1 commit
Commits
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
90 changes: 90 additions & 0 deletions src/copaw/cli/desktop_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from __future__ import annotations

import os
import signal
import socket
import subprocess
import sys
Expand Down Expand Up @@ -79,6 +80,89 @@ def _stream_reader(in_stream, out_stream) -> None:
pass


def _pid_exists(pid: int) -> bool:
"""Return True if *pid* exists and is accessible."""
try:
os.kill(pid, 0)
except ProcessLookupError:
return False
except PermissionError:
return True
return True


def _cleanup_stale_desktop_backends() -> list[int]:
"""Terminate stale packaged desktop backend processes.

We only target packaged app backends (inside ``*.app`` bundle) to avoid
affecting source/development processes like ``python -m copaw app --reload``.
"""
if sys.platform == "win32":
# Packaged desktop backend process pattern below is macOS/Linux-specific.
return []

stale_pids: list[int] = []
try:
ps_out = subprocess.check_output(
["ps", "-axo", "pid=,command="],
text=True,
)
except Exception:
return []
Comment on lines +110 to +111
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The except Exception: is too broad and can hide unexpected errors. It's better to catch specific exceptions that subprocess.check_output is known to raise, such as subprocess.CalledProcessError and FileNotFoundError. Also, consider logging the error for easier debugging, which will be helpful if process scanning fails for some reason.

Suggested change
except Exception:
return []
except (subprocess.CalledProcessError, FileNotFoundError) as e:
_log_desktop(f'[desktop] Failed to scan processes while cleaning up: {e}')
return []


current_pid = os.getpid()
for raw in ps_out.splitlines():
line = raw.strip()
if not line:
continue
parts = line.split(None, 1)
if len(parts) != 2:
continue
pid_str, cmd = parts
try:
pid = int(pid_str)
except ValueError:
continue
if pid == current_pid:
continue

# Only clean stale desktop-packaged backend processes.
if (
".app/Contents/Resources/env/bin/python" in cmd
and "-m uvicorn" in cmd
and "copaw.app._app:app" in cmd
):
stale_pids.append(pid)

cleaned: list[int] = []
for pid in stale_pids:
try:
os.kill(pid, signal.SIGTERM)
cleaned.append(pid)
except ProcessLookupError:
continue
except Exception:
continue
Comment on lines +142 to +145
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Using except Exception: is too broad. It's better to catch specific exceptions. In this case, besides ProcessLookupError, os.kill might raise a PermissionError if the current user doesn't have the rights to terminate the process. It's good practice to handle these specific cases and let other unexpected exceptions propagate. You can combine the except blocks for cleaner code.

Suggested change
except ProcessLookupError:
continue
except Exception:
continue
except (ProcessLookupError, PermissionError):
continue


if cleaned:
# Give processes a short grace period for graceful shutdown.
deadline = time.monotonic() + 2.0
while time.monotonic() < deadline:
if all(not _pid_exists(pid) for pid in cleaned):
break
time.sleep(0.1)

# Force kill any survivors.
for pid in cleaned:
if _pid_exists(pid):
try:
os.kill(pid, signal.SIGKILL)
except Exception:
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The except Exception: is too broad and silently ignores any error that occurs when trying to SIGKILL a process. It's better to catch specific exceptions like ProcessLookupError (if the process died just before the kill attempt) or PermissionError.

Suggested change
except Exception:
pass
except (ProcessLookupError, PermissionError):
pass


return cleaned


@click.command("desktop")
@click.option(
"--host",
Expand Down Expand Up @@ -107,6 +191,12 @@ def desktop_cmd(
window without conflicting with an existing CoPaw app instance.
"""

cleaned = _cleanup_stale_desktop_backends()
if cleaned:
_log_desktop(
f"[desktop] Cleaned stale desktop backend process(es): {cleaned}",
)

port = _find_free_port(host)
url = f"http://{host}:{port}"
click.echo(f"Starting CoPaw app on {url} (port {port})")
Expand Down
Loading