-
Notifications
You must be signed in to change notification settings - Fork 259
Fix multiprocessing compatibility for Python 3.14 #4244
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.
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
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a top-level wrapper function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
avocado_vt/plugins/vt_runner.py(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Static checks
|
Reference: https://docs.python.org/3.14/whatsnew/3.14.html Python 3.14 tightened pickling rules, so bound methods capturing self with Queue or Lock fail. Before PatchAfter patch - Python3.14After patch - Python3.13 |
Avoid passing bound VirtTest.runTest method to multiprocessing.Process, because Python 3.14 no longer allows pickling objects containing locks or queues. Use a small wrapper function that creates the VirtTest instance inside the child process. This preserves behavior and ensures cross-process safety. Signed-off-by: Misbah Anjum N <misanjumn@ibm.com>
81963c3 to
36487f6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
avocado_vt/plugins/vt_runner.py (2)
117-123: Improve failure signaling and satisfy Ruff BLE001.Catching a broad
Exceptionhere is pragmatic; add an inline Ruff suppression and enrich the FinishedMessage for easier triage.Apply:
- except Exception: # pylint: disable=broad-exception-caught - queue.put(messages.StderrMessage.get(traceback.format_exc())) - queue.put(messages.FinishedMessage.get("error")) + except Exception: # pylint: disable=broad-exception-caught # noqa: BLE001 + tb = traceback.format_exc() + queue.put(messages.StderrMessage.get(tb)) + queue.put( + messages.FinishedMessage.get( + "error", + fail_reason="VirtTest initialization failed", + class_name="VirtTest", + fail_class="Exception", + traceback=tb, + ) + ) returnThis keeps behavior while improving diagnostics and clearing the static‑analysis warning. [Based on static analysis hints]
174-183: Avoid hangs/zombies: check child liveness and join after finish.If the child exits unexpectedly before emitting a FinishedMessage, the loop can spin forever; also, the process is never joined. Add a dead‑child check and always reap the process after breaking.
- while True: + while True: time.sleep(RUNNER_RUN_CHECK_INTERVAL) - if queue.empty(): - yield messages.RunningMessage.get() + if queue.empty(): + if not process.is_alive(): + yield messages.StderrMessage.get( + "avocado-vt: child process exited without sending a FinishedMessage" + ) + yield messages.FinishedMessage.get("error") + break + yield messages.RunningMessage.get() else: message = queue.get() yield message if message.get("status") == "finished": break + # Reap the child process to avoid zombies + process.join(timeout=5) + if process.is_alive(): + process.terminate() + process.join(timeout=1)This keeps the polling model but prevents indefinite waits and cleans up the worker.
Run a quick kill test (send SIGKILL to the child) to confirm the runner now exits with an error instead of hanging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
avocado_vt/plugins/vt_runner.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.13.1)
avocado_vt/plugins/vt_runner.py
119-119: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: fedora_40 AVOCADO_SRC:avocado-framework<104.0 SETUP:-m pip install . VT_TYPE:qemu
- GitHub Check: Static checks
🔇 Additional comments (2)
avocado_vt/plugins/vt_runner.py (2)
164-165: Cancel message fix looks good.String now reads correctly and matches expected nrunner semantics.
168-173: Correct: use a top‑level wrapper asProcesstarget (Py 3.14 compatible).Switching to
run_vt_testavoids pickling a bound method capturing non‑picklables. This addresses the Python 3.14 regression.Please confirm the two previously failing tests still pass under Python 3.13 and 3.14 after this change.
harihare
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fix multiprocessing compatibility for Python 3.14
Avoid passing bound VirtTest.runTest method to multiprocessing.Process, because Python 3.14 no longer allows pickling objects containing locks or queues.
Use a small wrapper function that creates the VirtTest instance inside the child process. This preserves behavior and ensures cross-process safety.
Signed-off-by: Misbah Anjum N misanjum@linux.ibm.com
Summary by CodeRabbit
Bug Fixes
Refactor