Modernize project: migrate to pyproject.toml and refactor IPC #7
Conversation
…e docs - Replace setup.py/setup.cfg/pbr with pyproject.toml (PEP 621) - Drop pbr build dependency, use plain setuptools - Bump python_requires from >=3.7 to >=3.10 (3.7-3.9 are EOL) - Replace multiprocessing.Manager with Pipe (no extra server process) - Fix zombie process leak: call process.join() after process.kill() - Fix id(object()) key reuse risk by using uuid4 - Add .gitignore - Add GitHub Actions CI matrix (Python 3.10-3.14) - Update README: fix typo, add POSIX note, add "How it works" section - Use f-strings instead of % formatting - Use SPDX license expression per PEP 639 https://claude.ai/code/session_01Qdue58yru1buXnhnEgMgvW
There was a problem hiding this comment.
Pull request overview
Modernizes the project packaging and runtime behavior by moving to pyproject.toml, refactoring the IPC mechanism used by the kill_timeout decorator, and adding CI/documentation updates.
Changes:
- Migrates packaging from
setup.py/setup.cfgtopyproject.toml(setuptools backend) - Refactors IPC from
multiprocessing.Manager().dict()tomultiprocessing.Pipe() - Adds GitHub Actions CI and updates README formatting/usage docs
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Removes legacy setup.py entrypoint in favor of pyproject.toml |
| setup.cfg | Removes legacy setuptools metadata/config |
| pyproject.toml | Adds modern project/build metadata + pytest config |
| kill_timeout.py | Replaces Manager-based result passing with Pipe-based IPC |
| README.md | Updates docs formatting, examples, and adds “How it works” |
| .github/workflows/ci.yml | Adds CI matrix to run pytest across multiple Python versions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kill_timeout.py
Outdated
| parent_conn, child_conn = multiprocessing.Pipe() | ||
|
|
||
| def target(): | ||
| child_conn.send(return_error(func)(*args, **kwargs)) | ||
| child_conn.close() | ||
|
|
||
| process = multiprocessing.Process(target=target) |
There was a problem hiding this comment.
The target callable is a nested function/closure, which is not picklable under the spawn start method (notably the default on macOS). That contradicts the README’s “POSIX (Linux, macOS, etc.)” claim and will fail at runtime on macOS. Prefer a top-level target function (and pass func/args/kwargs/child_conn explicitly), or explicitly use a fork context where supported and documented.
| process.join(seconds) | ||
| if process.is_alive(): | ||
| # NOTE: killing the process without getting its return code via | ||
| # .join() would cause the zombie process to stay until the next | ||
| # multiprocessing.Process() run, it is acceptable, since it | ||
| # shouldn't end up with many zombie processes, and the | ||
| # alternatives like creating the daemon threads to catch the | ||
| # zombies don't look like a good fit | ||
| process.kill() | ||
| if id(key) in results: | ||
| result = results.pop(id(key)) | ||
| process.join() | ||
| parent_conn.close() | ||
| raise TimeoutError( | ||
| f"function {func.__name__} didn't complete in {seconds} seconds" | ||
| ) | ||
| if parent_conn.poll(): | ||
| result = parent_conn.recv() | ||
| parent_conn.close() |
There was a problem hiding this comment.
If the child process exits without sending anything (e.g., pickling failure, early crash before send, or IPC failure), the parent currently raises TimeoutError, which is inaccurate and makes debugging difficult. Consider checking process.exitcode after join() and/or handling EOFError from recv() to raise a more accurate error (e.g., ChildProcessError/RuntimeError) instead of reporting a timeout.
| parent_conn.close() | ||
| raise TimeoutError( | ||
| f"function {func.__name__} didn't complete in {seconds} seconds" | ||
| ) |
There was a problem hiding this comment.
If the child process exits without sending anything (e.g., pickling failure, early crash before send, or IPC failure), the parent currently raises TimeoutError, which is inaccurate and makes debugging difficult. Consider checking process.exitcode after join() and/or handling EOFError from recv() to raise a more accurate error (e.g., ChildProcessError/RuntimeError) instead of reporting a timeout.
kill_timeout.py
Outdated
| parent_conn, child_conn = multiprocessing.Pipe() | ||
|
|
||
| def target(): | ||
| child_conn.send(return_error(func)(*args, **kwargs)) | ||
| child_conn.close() |
There was a problem hiding this comment.
Pipe.send() can block if the payload is large enough to fill the OS pipe buffer, and the parent waits in join() before reading. This can deadlock or produce a false timeout (child stuck sending, parent waiting to join). A more robust approach is to receive while waiting (poll loop with remaining timeout), or use multiprocessing.Queue (which uses a feeder thread) to avoid blocking the child on large writes.
|
|
||
| process = multiprocessing.Process(target=target) | ||
| process.start() | ||
| process.join(seconds) |
There was a problem hiding this comment.
Pipe.send() can block if the payload is large enough to fill the OS pipe buffer, and the parent waits in join() before reading. This can deadlock or produce a false timeout (child stuck sending, parent waiting to join). A more robust approach is to receive while waiting (poll loop with remaining timeout), or use multiprocessing.Queue (which uses a feeder thread) to avoid blocking the child on large writes.
kill_timeout.py
Outdated
| def target(): | ||
| child_conn.send(return_error(func)(*args, **kwargs)) | ||
| child_conn.close() | ||
|
|
||
| process = multiprocessing.Process(target=target) | ||
| process.start() |
There was a problem hiding this comment.
The pipe endpoints aren’t fully closed in both processes. In the parent, child_conn should be closed promptly after starting the child; in the child, the inherited parent_conn should be closed before/after sending. Properly closing the unused ends avoids descriptor leaks and helps ensure EOF/cleanup semantics behave reliably.
| def target(): | |
| child_conn.send(return_error(func)(*args, **kwargs)) | |
| child_conn.close() | |
| process = multiprocessing.Process(target=target) | |
| process.start() | |
| def target(): | |
| parent_conn.close() | |
| child_conn.send(return_error(func)(*args, **kwargs)) | |
| child_conn.close() | |
| process = multiprocessing.Process(target=target) | |
| process.start() | |
| child_conn.close() |
.github/workflows/ci.yml
Outdated
| - uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: ${{ matrix.python-version }} | ||
| - run: pip install tblib pytest |
There was a problem hiding this comment.
CI is not installing the package itself, so it doesn’t validate that pyproject.toml packaging/install metadata works (nor that dependencies are resolved via the project metadata). Consider installing the project (e.g., pip install -e . or pip install .) before running tests, and only install pytest as a test dependency if it’s not declared elsewhere.
| - run: pip install tblib pytest | |
| - run: pip install tblib pytest | |
| - run: pip install . |
- Use explicit fork context (get_context("fork")) so the decorator works
on macOS where the default start method is "spawn"
- Extract target() to a top-level _target() function (picklable)
- Close child_conn in parent after process.start(), close parent_conn
in child via _target's finally block (fix descriptor leaks)
- Raise ChildProcessError instead of TimeoutError when child exits
without sending data (crash, signal, pickling failure)
- Check process.exitcode to distinguish crashes from timeouts
- Document large-return-value pipe buffer limitation in docstring
- CI: install package via `pip install .` to validate pyproject.toml
https://claude.ai/code/session_01Qdue58yru1buXnhnEgMgvW
2 participants
Summary
This PR modernizes the kill-timeout project by migrating to a
pyproject.toml-based build system, refactoring the inter-process communication mechanism, and improving documentation and CI/CD setup.Key Changes
setup.pyandsetup.cfgwith modernpyproject.tomlusing setuptools backendmultiprocessing.Manager()withmultiprocessing.Pipe()for more efficient inter-process communication.github/workflows/ci.yml) to test against Python 3.10-3.14pyproject.tomlwith proper dependencies, classifiers, and project URLsrequirements.txtand.gitignorenow properly configuredImplementation Details
The refactored
kill_timeoutdecorator now usesmultiprocessing.Pipe()instead ofManager().dict():poll()before attempting to receiveclose()calls on pipe connectionshttps://claude.ai/code/session_01Qdue58yru1buXnhnEgMgvW