fix(memory): prevent close() race and enforce closed state in AsyncSQLiteSession

[hsusul]

Summary

Fixes a concurrency race in AsyncSQLiteSession.close() and enforces closed-state checks across session operations.

• Problem 1 (Race on close): AsyncSQLiteSession.close() evaluated if self._connection is None: outside self._lock. When concurrent tasks (e.g. during application shutdown or teardown) called close(), both passed the None check. The first task acquired self._lock, closed the connection, and set self._connection = None. The second task then acquired self._lock and executed await self._connection.close(), raising AttributeError: 'NoneType' object has no attribute 'close'.
• Problem 2 (Closed state enforcement): AsyncSQLiteSession lacked a self._closed state flag (unlike SQLiteSession). Consequently, invoking get_items(), add_items(), pop_item(), or clear_session() after close() re-initialized a database connection instead of raising RuntimeError("AsyncSQLiteSession is closed").

Solution:

1. Added self._closed flag to AsyncSQLiteSession.
2. Updated close() to acquire self._lock, check self._closed, mark self._closed = True, and safely close self._connection if active.
3. Updated _get_connection() to check self._closed both outside and inside self._init_lock, raising RuntimeError("AsyncSQLiteSession is closed").

Test plan

• Added test_async_sqlite_session_concurrent_close in tests/extensions/memory/test_async_sqlite_session.py to verify that concurrent (asyncio.gather) and repeated calls to close() succeed cleanly.
• Added test_async_sqlite_session_closed_operations_raise_runtime_error to verify that calling get_items(), add_items(), pop_item(), or clear_session() on a closed session raises RuntimeError("AsyncSQLiteSession is closed").
• Verified formatting, linting, typechecking, and tests:
  • make format (passed, 841 files unchanged)
  • make lint (passed, 0 errors)
  • uv run pyright src/agents/extensions/memory/async_sqlite_session.py tests/extensions/memory/test_async_sqlite_session.py (passed, 0 errors)
  • uv run pytest tests/extensions/memory/test_async_sqlite_session.py -v (20 passed)
  • git diff --check (passed, 0 warnings)

Issue number

Closes #3983

Checks

• I've added new tests, if relevant
• I've run .agents/skills/code-change-verification/scripts/run.sh
• I've confirmed all verification steps pass
• If using Codex, I've run /review before submitting this PR

[seratch]

Thanks for the contribution. The _closed state is appropriate here: checking _connection only inside the lock fixes concurrent close() calls, but it still allows an operation waiting behind close() to create a new connection immediately after shutdown completes.

Before merging, please add a controlled interleaving regression test where close() acquires the session lock first, an operation starts while close is paused, and close then completes. The test should verify that the waiting operation raises RuntimeError and that no connection is recreated. Please keep the _closed state and the existing post-close checks. This will lock down the terminal lifecycle behavior that the patch introduces.

[hsusul]

Added test_async_sqlite_session_close_rejects_operation_waiting_on_lock to lock down the terminal lifecycle. It pauses close() while it holds the session lock (during connection close), starts get_items() so it waits on that lock, then lets close finish. The waiter raises RuntimeError("AsyncSQLiteSession is closed"), _connection stays None, and no new aiosqlite.connect occurs. Kept the _closed flag and existing post-close checks unchanged.

[seratch]

Thanks for adding the controlled close-versus-operation interleaving test. The scenario and final assertions now cover the requested lifecycle behavior.

One test-lifecycle issue remains: assertions run while close() is intentionally paused, before release_close is set. If any of those assertions fail, the paused close task and the aiosqlite worker thread can remain alive, causing the regression test to hang instead of failing cleanly. Please guarantee release and task cleanup with try/finally, use a test-owned event instead of inspecting asyncio.Lock._waiters, and keep the synchronization waits bounded. The runtime implementation otherwise looks ready.

[hsusul]

Updated the interleaving test lifecycle: all mid-pause assertions are inside try/finally so release_close is always set and both tasks are cleaned up; waiting is signaled through a test-owned event on lock.acquire instead of asyncio.Lock._waiters; and the synchronization waits are bounded with asyncio.wait_for.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 77aa05344e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Reject empty writes after closure

When a caller invokes add_items([]) after close(), the method returns through its empty-list fast path before reaching this new _get_connection() guard, so a closed session silently accepts that public operation while reads and non-empty writes raise RuntimeError. Check the closed state before the empty-list return and cover this unsupported post-close case.

AGENTS.md reference: AGENTS.md:L62-L64

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Terminate the new comments with periods

These two newly added comments are complete sentences but omit terminal periods, contrary to the repository's explicit comment-formatting requirement; append periods to both comments.

AGENTS.md reference: AGENTS.md:L201-L204

Useful? React with 👍 / 👎.