Skip to content

TEST: Join SQLite cleanup workers before leak checks - #3362

Merged
Roman Lutz (romanlutz) merged 2 commits into
microsoft:mainfrom
richlundeen:richlundeen-sqlite-fixture-cleanup
Oct 20, 2026
Merged

Roman Lutz (romanlutz) merged 2 commits into
microsoft:mainfrom
richlundeen:richlundeen-sqlite-fixture-cleanup

Conversation

@richlundeen

Copy link
Copy Markdown
Contributor

Description

Fix the intermittent GC assertion in test_sqlite_instance_does_not_retain_process_exit_cleanup. It failed in the Windows Python 3.13 dev merge-group job 118365076379 while checking #3206.

dispose_engine_async awaits run_legacy_sync_async(self._dispose_sync_engine), which uses asyncio.to_thread. A thread-pool future can notify the event loop before its worker deletes the work item. That work item still holds the bound disposal method and thus the memory instance. gc.collect() cannot collect a live reference. This is a test lifetime race, not a process-exit hook leak or a result of #3206's seed-storage changes.

Run the existing fixture lifecycle in its own asyncio.Runner. Exit the runner to join its default executor before the unchanged GC and weak-reference assertion. Keep the checks that the real cleanup method is restored. No production code, shared fixture, timeout, retry, or leak assertion changes. This does not duplicate #3361 or #3344.

Tests and Documentation

  • uv sync --frozen --extra all — passed; lockfile unchanged.
  • uv run --no-sync pytest tests\unit\memory\test_sqlite_fixtures.py -q — passed, 14 tests on Windows Python 3.14.2.
  • $env:UV_NO_SYNC='1'; uv run --no-sync pre-commit run --all-files — all hooks passed.
  • git --no-pager diff --check — passed.
  • Deterministic in-process race harness: pause ThreadPoolExecutor immediately after _WorkItem.run completes; release the worker when executor shutdown starts. The old GC assertion failed 10/10 times. The corrected test passed 10/10 times. Direct referrers included the bound disposal method and its coroutine.
  • Leak-detection control: register a real bound disposal method with atexit after disposal. The corrected test still fails its weak-reference assertion, as required. The harness unregisters the callback afterward.

No documentation or notebook behavior changes; JupyText was not run. The full unit suite and the exact CI Python 3.13 environment were not run locally.

Run the fixture GC regression in an isolated asyncio.Runner so executor work items release bound disposal methods before collection. Keep the weak-reference assertion and real cleanup checks.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: ac1d8921-cfc7-4a1c-bf94-cc7c7bf51566
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Oct 20, 2026
Merged via the queue into microsoft:main with commit 9aa9c82 Oct 20, 2026
60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants