[AUTO-INF-6] Missing SQLAlchemy engine/session cleanup handlers in BDD step files cause resource leaks on scenario failure #10294

New issue

Open

opened 2026-04-18 08:15:59 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-18 08:15:59 +00:00

Owner

Metadata

Field	Value
Branch	`test/auto-inf-6-sqlalchemy-cleanup-handlers`
Commit Message	`test(infra): register SQLAlchemy engine/session cleanup handlers in BDD step files to prevent resource leaks`
Milestone	v3.5.0
Parent Epic	—

Background and Context

Multiple BDD step files in features/steps/ create SQLAlchemy engines and sessions during scenario setup but do not register cleanup handlers to dispose of them when scenarios end (especially on failure). This causes resource leaks — open file handles, locked SQLite files, and unreleased memory — that can accumulate across a long test run and contribute to CI instability (related to issue #2850).

Evidence — three affected files:

1. `features/steps/session_persistence_steps.py` (lines 35–50)

def _setup_service(context: Context) -> None:
    """Set up an in-memory database and service for testing."""
    engine = create_engine("sqlite:///:memory:", echo=False)
    Base.metadata.create_all(engine)
    factory = sessionmaker(bind=engine, expire_on_commit=False)
    db_session = factory()
    context._db_session = db_session
    context._db_engine = engine
    # NO CLEANUP HANDLER REGISTERED

The engine and session are stored on context but no after_scenario cleanup handler is registered. If the scenario fails before the test's own teardown step, the engine is never disposed and the session is never closed.

2. `features/steps/concurrency_steps.py` (lines 45–65)

def _build_lock_service(context: Context) -> LockService:
    """Build a LockService with an in-memory SQLite backend."""
    engine = create_engine(
        "sqlite:///:memory:",
        echo=False,
        future=True,
        connect_args={"check_same_thread": False},
    )
    # ... setup code ...
    context.lock_engine = engine
    context.lock_session_factory = factory
    return LockService(session_factory=factory)
    # NO CLEANUP HANDLER REGISTERED

The engine is stored on context.lock_engine but never disposed. In parallel test runs, this can exhaust SQLite connection limits.

3. `features/steps/plan_persistence_steps.py` (lines 95–110)

def _setup_db(context: Context, *, file_based: bool = False) -> None:
    """Create a SQLite DB and attach repos to context."""
    if file_based:
        tmp = tempfile.mktemp(suffix=".db")
        db_url = f"sqlite:///{tmp}"
        context._pp_db_path = tmp
    else:
        db_url = "sqlite://"
        context._pp_db_path = None
    engine = create_engine(db_url, echo=False)
    Base.metadata.create_all(engine)
    sm = sessionmaker(bind=engine)
    session = sm()
    context._pp_session = session
    # Cleanup only registered if _cleanup_handlers exists
    if not hasattr(context, "_cleanup_handlers"):
        context._cleanup_handlers = []
    context._cleanup_handlers.append(lambda: _teardown_db(context))

The cleanup handler registration is conditional on _cleanup_handlers not existing. However, the _cleanup_handlers list is initialized in before_scenario in environment.py. If _setup_db is called before before_scenario completes (e.g., in a @given step that runs before the hook), the handler may not be registered.

Impact

Resource leaks: SQLAlchemy engines hold open file handles and connection pools. In a 587-scenario test run, unreleased engines accumulate and can exhaust OS file descriptor limits.
CI instability: Leaked SQLite file handles can cause disk I/O error failures in subsequent scenarios that try to access the same database file (related to issue #2850).
Parallel test failures: In behave-parallel runs, leaked connections from one worker can block another worker from accessing the same in-memory database.

Expected Behavior

All SQLAlchemy engines and sessions created in step files should be properly disposed of after each scenario, even if the scenario fails. The correct pattern is:

def _setup_service(context: Context) -> None:
    """Set up an in-memory database and service for testing."""
    engine = create_engine("sqlite:///:memory:", echo=False)
    Base.metadata.create_all(engine)
    factory = sessionmaker(bind=engine, expire_on_commit=False)
    db_session = factory()
    context._db_session = db_session
    context._db_engine = engine
    
    # Register cleanup handler
    if not hasattr(context, "_cleanup_handlers"):
        context._cleanup_handlers = []
    context._cleanup_handlers.append(lambda: _cleanup_service(context))

def _cleanup_service(context: Context) -> None:
    """Dispose engine and close session."""
    if hasattr(context, "_db_session") and context._db_session:
        context._db_session.close()
    if hasattr(context, "_db_engine") and context._db_engine:
        context._db_engine.dispose()

Acceptance Criteria

features/steps/session_persistence_steps.py: _setup_service() registers a cleanup handler that closes context._db_session and disposes context._db_engine
features/steps/concurrency_steps.py: _build_lock_service() registers a cleanup handler that disposes context.lock_engine
features/steps/plan_persistence_steps.py: _setup_db() cleanup handler registration is unconditional (not dependent on _cleanup_handlers pre-existing)
All nox sessions pass without regressions
Coverage >= 97%

Subtasks

Audit session_persistence_steps.py for all SQLAlchemy engine/session creation; add cleanup handlers
Audit concurrency_steps.py for all SQLAlchemy engine creation; add cleanup handlers
Audit plan_persistence_steps.py for conditional cleanup handler registration; make unconditional
Audit remaining step files with sqlite references for missing cleanup handlers
Run nox -s unit_tests to confirm no regressions
Run nox -s coverage_report to confirm coverage >= 97%

Definition of Done

This issue should be closed when:

All SQLAlchemy engines and sessions in affected step files have registered cleanup handlers
Cleanup handlers are unconditionally registered (not conditional on pre-existing state)
All nox stages pass
Coverage >= 97%

Duplicate Check

Search 1 — open issues, keywords "session leak", "cleanup handler", "engine dispose", "connection leak":
Searched open issues pages 1–12 for "session leak", "cleanup handler", "engine dispose", "connection leak", "db session", "database session", "session close", "after_scenario", "teardown", "resource leak". No matches found.

Search 2 — cross-area analysis:

#10082 (context.temp_dir cleanup): covers missing after_scenario cleanup for temp directories, not SQLAlchemy engine/session disposal. Different issue.
#10245 (module-level ULID generation): covers module-level ULID generation at import time, not session cleanup. Different issue.
#10244 (environment.py refactoring): covers extracting DB init helpers from environment.py, not step file cleanup handlers. Different issue.
#7087 (tempfile.mktemp() race condition): covers tempfile.mktemp() race condition in environment.py, not SQLAlchemy session cleanup. Different issue.

Search 3 — closed issues, keywords "session leak", "cleanup handler", "engine dispose":
Searched closed issues pages 1–11 for same keywords. No matches found.

Search 4 — keywords "SQLAlchemy cleanup", "engine.dispose", "session.close step":
No existing open or closed issue covers the specific pattern of missing SQLAlchemy engine/session cleanup handlers in BDD step files.

Search 5 — uncertainty check:
This finding is clearly distinct from all existing issues. The specific problem (missing cleanup handlers for SQLAlchemy engines and sessions in step files) is not addressed by any existing issue. Confident this is a new gap.

Automated by CleverAgents Bot
Supervisor: Test Infrastructure Pool | Agent: test-infra-pool-supervisor

## Metadata | Field | Value | |---|---| | **Branch** | `test/auto-inf-6-sqlalchemy-cleanup-handlers` | | **Commit Message** | `test(infra): register SQLAlchemy engine/session cleanup handlers in BDD step files to prevent resource leaks` | | **Milestone** | v3.5.0 | | **Parent Epic** | — | --- ## Background and Context Multiple BDD step files in `features/steps/` create SQLAlchemy engines and sessions during scenario setup but do **not** register cleanup handlers to dispose of them when scenarios end (especially on failure). This causes resource leaks — open file handles, locked SQLite files, and unreleased memory — that can accumulate across a long test run and contribute to CI instability (related to issue #2850). **Evidence — three affected files:** ### 1. `features/steps/session_persistence_steps.py` (lines 35–50) ```python def _setup_service(context: Context) -> None: """Set up an in-memory database and service for testing.""" engine = create_engine("sqlite:///:memory:", echo=False) Base.metadata.create_all(engine) factory = sessionmaker(bind=engine, expire_on_commit=False) db_session = factory() context._db_session = db_session context._db_engine = engine # NO CLEANUP HANDLER REGISTERED ``` The engine and session are stored on `context` but no `after_scenario` cleanup handler is registered. If the scenario fails before the test's own teardown step, the engine is never disposed and the session is never closed. ### 2. `features/steps/concurrency_steps.py` (lines 45–65) ```python def _build_lock_service(context: Context) -> LockService: """Build a LockService with an in-memory SQLite backend.""" engine = create_engine( "sqlite:///:memory:", echo=False, future=True, connect_args={"check_same_thread": False}, ) # ... setup code ... context.lock_engine = engine context.lock_session_factory = factory return LockService(session_factory=factory) # NO CLEANUP HANDLER REGISTERED ``` The engine is stored on `context.lock_engine` but never disposed. In parallel test runs, this can exhaust SQLite connection limits. ### 3. `features/steps/plan_persistence_steps.py` (lines 95–110) ```python def _setup_db(context: Context, *, file_based: bool = False) -> None: """Create a SQLite DB and attach repos to context.""" if file_based: tmp = tempfile.mktemp(suffix=".db") db_url = f"sqlite:///{tmp}" context._pp_db_path = tmp else: db_url = "sqlite://" context._pp_db_path = None engine = create_engine(db_url, echo=False) Base.metadata.create_all(engine) sm = sessionmaker(bind=engine) session = sm() context._pp_session = session # Cleanup only registered if _cleanup_handlers exists if not hasattr(context, "_cleanup_handlers"): context._cleanup_handlers = [] context._cleanup_handlers.append(lambda: _teardown_db(context)) ``` The cleanup handler registration is conditional on `_cleanup_handlers` not existing. However, the `_cleanup_handlers` list is initialized in `before_scenario` in `environment.py`. If `_setup_db` is called before `before_scenario` completes (e.g., in a `@given` step that runs before the hook), the handler may not be registered. --- ## Impact 1. **Resource leaks**: SQLAlchemy engines hold open file handles and connection pools. In a 587-scenario test run, unreleased engines accumulate and can exhaust OS file descriptor limits. 2. **CI instability**: Leaked SQLite file handles can cause `disk I/O error` failures in subsequent scenarios that try to access the same database file (related to issue #2850). 3. **Parallel test failures**: In `behave-parallel` runs, leaked connections from one worker can block another worker from accessing the same in-memory database. --- ## Expected Behavior All SQLAlchemy engines and sessions created in step files should be properly disposed of after each scenario, even if the scenario fails. The correct pattern is: ```python def _setup_service(context: Context) -> None: """Set up an in-memory database and service for testing.""" engine = create_engine("sqlite:///:memory:", echo=False) Base.metadata.create_all(engine) factory = sessionmaker(bind=engine, expire_on_commit=False) db_session = factory() context._db_session = db_session context._db_engine = engine # Register cleanup handler if not hasattr(context, "_cleanup_handlers"): context._cleanup_handlers = [] context._cleanup_handlers.append(lambda: _cleanup_service(context)) def _cleanup_service(context: Context) -> None: """Dispose engine and close session.""" if hasattr(context, "_db_session") and context._db_session: context._db_session.close() if hasattr(context, "_db_engine") and context._db_engine: context._db_engine.dispose() ``` --- ## Acceptance Criteria - [ ] `features/steps/session_persistence_steps.py`: `_setup_service()` registers a cleanup handler that closes `context._db_session` and disposes `context._db_engine` - [ ] `features/steps/concurrency_steps.py`: `_build_lock_service()` registers a cleanup handler that disposes `context.lock_engine` - [ ] `features/steps/plan_persistence_steps.py`: `_setup_db()` cleanup handler registration is unconditional (not dependent on `_cleanup_handlers` pre-existing) - [ ] All nox sessions pass without regressions - [ ] Coverage >= 97% --- ## Subtasks - [ ] Audit `session_persistence_steps.py` for all SQLAlchemy engine/session creation; add cleanup handlers - [ ] Audit `concurrency_steps.py` for all SQLAlchemy engine creation; add cleanup handlers - [ ] Audit `plan_persistence_steps.py` for conditional cleanup handler registration; make unconditional - [ ] Audit remaining step files with `sqlite` references for missing cleanup handlers - [ ] Run `nox -s unit_tests` to confirm no regressions - [ ] Run `nox -s coverage_report` to confirm coverage >= 97% --- ## Definition of Done This issue should be closed when: - [ ] All SQLAlchemy engines and sessions in affected step files have registered cleanup handlers - [ ] Cleanup handlers are unconditionally registered (not conditional on pre-existing state) - [ ] All nox stages pass - [ ] Coverage >= 97% --- ### Duplicate Check **Search 1 — open issues, keywords "session leak", "cleanup handler", "engine dispose", "connection leak":** Searched open issues pages 1–12 for "session leak", "cleanup handler", "engine dispose", "connection leak", "db session", "database session", "session close", "after_scenario", "teardown", "resource leak". No matches found. **Search 2 — cross-area analysis:** - #10082 (`context.temp_dir` cleanup): covers missing `after_scenario` cleanup for temp directories, not SQLAlchemy engine/session disposal. Different issue. - #10245 (module-level ULID generation): covers module-level ULID generation at import time, not session cleanup. Different issue. - #10244 (environment.py refactoring): covers extracting DB init helpers from environment.py, not step file cleanup handlers. Different issue. - #7087 (tempfile.mktemp() race condition): covers `tempfile.mktemp()` race condition in environment.py, not SQLAlchemy session cleanup. Different issue. **Search 3 — closed issues, keywords "session leak", "cleanup handler", "engine dispose":** Searched closed issues pages 1–11 for same keywords. No matches found. **Search 4 — keywords "SQLAlchemy cleanup", "engine.dispose", "session.close step":** No existing open or closed issue covers the specific pattern of missing SQLAlchemy engine/session cleanup handlers in BDD step files. **Search 5 — uncertainty check:** This finding is clearly distinct from all existing issues. The specific problem (missing cleanup handlers for SQLAlchemy engines and sessions in step files) is not addressed by any existing issue. Confident this is a new gap. --- **Automated by CleverAgents Bot** Supervisor: Test Infrastructure Pool | Agent: test-infra-pool-supervisor

HAL9000 added the

labels

2026-04-18 08:20:25 +00:00