-
Notifications
You must be signed in to change notification settings - Fork 72
fix: add Windows-specific LMDB cleanup handling #1144
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: main
Are you sure you want to change the base?
Conversation
53a2e3b to
27a8eee
Compare
- Add explicit env.sync(force=True) before close to flush writes - Implement exponential backoff retry (50ms, 100ms, 200ms, 400ms, 800ms) - Catch PermissionError on Windows during cleanup - Ensures file locks are released before directory removal - Fixes test_essr_stream and test_essr_mbx PermissionError on Windows CI Resolves: Pre-existing Windows CI test failure Related: PR WebOfTrust#1143
Local Windows Testing ResultsI tested the Windows LMDB fix locally and got unexpected results that warrant discussion before merging. Test Environment
ResultsAll LMDB-related tests PASSED ✅: pytest tests/app/test_storing.py::test_mailboxing -v
# 1 passed in 1.57s
pytest tests/db/test_dbing.py -v
# 4 passed in 0.89s
pytest tests/app/test_habbing.py -v
# 10 passed in 6.90sImportant Caveat
|
- Downloads libsodium 1.0.20 stable MSVC build - Extracts and copies DLL to System32 (GitHub Actions has admin) - Fixes missing dependency that prevented pysodium imports on Windows CI - Resolves: Pre-existing Windows CI import failures
27a8eee to
e60b1c4
Compare
- Remove complex error type checking that may have been failing - Retry on ANY PermissionError/OSError during Windows cleanup - Increase base delay to 100ms (was 50ms) - Store last_error to ensure proper exception re-raising - More aggressive retry stance: assume all errors are lock-related The previous logic was too conservative in detecting lock errors, causing immediate failures instead of retries.
- Change from '>=3.12.6' to exact '3.14.2' - Ensures CI uses same Python version as successful local tests - Eliminates version variance as a potential cause of test failures
- Windows blocks access to ports 5000-5999 with permission error - Created windows_ports.py utility for cross-platform port mapping - Updated test_forwarding.py to use available ports (8000+) - Resolves: RuntimeError on HTTP server creation - Resolves: AttributeError on socket.accept() (was NoneType) This fixes the actual root cause discovered in native Windows testing: the failures were NOT LMDB cleanup issues, but Windows network port access restrictions.
|
Converting to draft - shifting focus per team guidance After multiple iterations attempting to fix Windows CI failures (LMDB cleanup, port binding issues), tests are still failing. Per team discussion, switching focus to Sphinx documentation work which has no platform-specific issues. What was attempted: Windows LMDB retry logic with exponential backoff Tests still fail with similar errors |
Resolves: Pre-existing Windows CI test failure
Related: PR #1143