fix: get_minicroft leaks MiniCroft on BaseException during boot - #134
fix: get_minicroft leaks MiniCroft on BaseException during boot#134JarbasAl wants to merge 1 commit into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Checking in! Here's how the automated tests are looking. 🧐I've aggregated the results of the automated checks for this PR below. 🔍 LintHere's the lowdown on the latest automated check. 📉 ❌ ruff: issues found — see job log 📋 Repo HealthScanning for any signs of code rot or decay. 🍄 ✅ All required files present. Latest Version: ✅ 🏷️ Release PreviewThe release notes are being proofread by the gnomes. 🍄 Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
⚖️ License CheckEnsuring our EULA (if any) is still valid. 📑 ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔒 Security (pip-audit)I've checked the vulnerability database for hits. 🎯 ✅ No known vulnerabilities found (79 packages scanned). 📊 CoverageA deep dive into the sea of test results. 🌊 ❌ 59.0% total coverage Files below 80% coverage (15 files)
Full report: download the 🔨 Build TestsI've finished the digital carpentry on this PR. 🔨
❌ 3.10: Install OK, tests failed Providing clarity through automated analysis 🔍 |
get_minicroft()wrapscroft.start()and the READY poll inexcept Exception: croft.stop(); raise.pytest-timeout'sFailedexception andKeyboardInterruptboth derive fromBaseException, notException, so either one raised while waiting for READY skipscroft.stop()and leaks the started MiniCroft process (and its background threads) instead of being cleaned up.Fix: catch
BaseExceptioninstead ofExceptioninget_minicroft, so cleanup runs regardless of which exception type interrupts the boot, then re-raise as before.Added a regression test (
test_basedexception_during_boot_still_stops_croftintest/unittests/test_minicroft.py) that patchesMiniCroft.startto raiseKeyboardInterruptand assertsMiniCroft.stopis called exactly once and the exception still propagates. Verified locally: the test fails against the pre-fix code (except Exception) and passes after changing it toexcept BaseException.