# Resource cleanup in LDR

This document captures how LDR manages process-level resources (DB
connections, HTTP clients, file descriptors, threads) and the reasoning
trail behind the current model. It exists because file-descriptor
exhaustion has been a recurring class of bug in LDR, and the *journey*
of fixing it — what's been tried, what worked, what was ruled out — is
not reconstructable from `git log` alone.

If you're contributing code that holds a network connection, a database
session, an LLM client, or a thread, read this before adding `__del__`,
`weakref.finalize`, or a context manager.

---

## Current model

### Database connections

- **One shared per-user `QueuePool`.** No per-thread engines. Pool
  sizing: `pool_size=20`, `max_overflow=40`, with periodic `dispose()`
  every 30 minutes.
- **SQLCipher is decrypted once per connection-open.** `PRAGMA key`
  takes ~0.2 ms; pool reuse keeps that off the hot path.
- Engines are created at login, closed at logout (or process exit via
  the registered `atexit` shutdown).
- Background threads (research workers, metric writers, news scheduler
  jobs) use the same per-user pool — they no longer maintain a separate
  thread-engine system.

See [ADR-0004](../decisions/0004-nullpool-for-sqlcipher.md) for the
QueuePool-vs-NullPool decision and PR #3441 for the per-thread-engine
removal.

### LLM wrappers

LDR wraps every LLM in `ProcessingLLMWrapper` (and optionally
`RateLimitedLLMWrapper`) so that callers see a uniform interface and
the project owns the close path:

```
caller -> ProcessingLLMWrapper.close()
       -> _close_base_llm(base_llm) in utilities/llm_utils.py
       -> for ChatOllama:
            sync httpx client (ollama.Client._client) .close()
            async httpx client (ollama.AsyncClient._client) .aclose()
       -> for ChatOpenAI / ChatAnthropic:
            no close (those use @lru_cache'd shared httpx clients)
```

Key invariants:

- `ChatOllama` is the *only* provider where `_close_base_llm()` actually
  closes anything. ChatOpenAI and ChatAnthropic share LRU-cached httpx
  clients across instances; closing them would break other live LLMs.
- Both `_client` (sync) **and** `_async_client` (async) are released —
  the async side is exercised by every `ainvoke()` call (langgraph
  agents, modular strategies). Closing only the sync side leaks the
  async transport per call (root cause of #3816).
- The function is idempotent via an `_ldr_closed` sentinel on the inner
  httpx clients.
- The async close uses `asyncio.run(client.aclose())` only when no
  event loop is currently running. When called from inside async code
  it skips and leaves the close to the loop's owner.

### Search engines

- `BaseSearchEngine.close()` is the single entry point and **cascades**
  into `_preview_filters` and `_content_filters`. That cascade is what
  releases per-engine LLMs (e.g., `JournalReputationFilter.model`),
  SearXNG sessions, and other filter-held resources.
- Search-engine cleanup happens at the per-research finally block in
  `web/services/research_service.py:run_research_process()` and at the
  programmatic API entry points in `api/research_functions.py`.
- The `_owns_llm` flag pattern (introduced in #2712) tracks whether a
  filter or engine constructed its own LLM (and thus owns it) versus
  borrowed one from a caller (and must not close it).

### Thread lifecycle

- `@thread_cleanup` (decorator on `run_research_process` and similar
  workers) ensures thread-local DB sessions are released even on
  abnormal exits.
- `cleanup_current_thread()` is called from Flask teardown, the queue
  processor, the auth flow, and the RAG routes — six tier-1 paths in
  total.
- Background threads are daemon threads; the process exit handles any
  thread that did not clean up gracefully.

### Conventions

- **Use `safe_close(resource, "human name")`** from
  `utilities/resource_utils.py` for every cleanup. Never bare `.close()`
  in a `finally` (it can mask the original exception).
- **Prefer `try/finally` over `__del__`**. Python doesn't guarantee
  finalization order at interpreter exit; `__del__` interacts subtly
  with reference cycles and `weakref`.
- **Track ownership explicitly with `_owns_llm` (or analogous flag)**
  when a class accepts an injected resource that may or may not be its
  own.
- **News fragments (`changelog.d/<id>.bugfix.md`) are required for any
  user-visible cleanup behavior change** — see `changelog.d/README.md`.

---

## How to close X correctly

| You're holding | Do this |
| --- | --- |
| A `ChatOllama` (raw or wrapped) | Call `wrapper.close()` in a `finally`, or pass to `safe_close(wrapper, "...")`. The wrapper chain handles both sync and async httpx clients. |
| A search engine you constructed | `safe_close(engine, "...")` in `finally`. The engine's `close()` cascades into preview/content filters. |
| A holder class with an LLM | Add a `close()` method, gate the LLM close on `self._owns_llm`, document who calls it. Don't add `__del__`. |
| A long-lived service holder (news scheduler, etc.) | Wrap construction in `try/finally` at the cycle boundary. Don't store the LLM if you can recreate it cheaply. |
| A DB session | Use `with get_user_db_session(username) as session:`. Don't bypass via `get_settings_manager(username=...)` without `owns_session=False` (see #3023). |
| An asyncio event loop | Use the existing one. If you genuinely need a new one (background thread fallback), call `loop.close()` in a `finally` — see `news_strategy.py` for the reference pattern (post-#3018). |

---

## Anti-patterns

These look reasonable but break specific things in this codebase:

- **Adding `__del__` to a class with `close()`.** At interpreter exit
  the `logger`, `httpx`, and event-loop modules may already be torn
  down. `__del__` can run after them and raise. Use explicit close in
  a `finally` instead.
- **Closing a shared httpx client.** ChatOpenAI / ChatAnthropic share
  one httpx pool across instances via `@lru_cache`. Closing it kills
  every other live LLM in the same process. The Ollama check in
  `_close_base_llm` exists exactly to gate this.
- **Truthy idempotency sentinels on Mock objects.** `Mock()` without a
  `spec` auto-generates child Mocks for any attribute access, so
  `getattr(client, "_ldr_closed", False)` returns a truthy Mock and
  short-circuits the close. Always use `is True` / `is None` checks
  for sentinels — see the pattern in `_close_base_llm`.
- **Skipping `super().close()` in a search-engine subclass.**
  `BaseSearchEngine.close()` is what cascades into preview/content
  filters. Override it without calling super and you leak every
  filter's resources (this was a Copilot finding on #3818).
- **Treating `asyncio.run()` as safe inside an event loop.** It raises
  `RuntimeError` if called from a thread that already has a running
  loop. The pattern in `_close_base_llm` is: detect a running loop
  with `get_running_loop()`, skip the async close in that branch (the
  loop owner will close), only call `asyncio.run` in the no-loop case.

---

## History

The FD-leak campaign spans roughly four months of iterative work. Each
fix narrowed the remaining surface; each subsequent issue was found in
a corner the previous wave hadn't touched.

### Wave 1 — initial leak inventory (Jan 2026)

- **#1832, #1849, #1856, #1860** — first comprehensive sweep. Identified
  seven distinct leak sources: `auth_db` engine, `download_management`
  DB, search cache, subprocess zombies, HTTP sessions in
  `SemanticScholarSearchEngine` and `BaseDownloader`, Socket.IO threads.
  Established context-manager + `try/finally` patterns. Added a
  pre-commit hook to catch missing cleanup at commit time.

### Wave 2 — thread-local engine accumulation (Mar 2026)

- **#2495** — diagnosed that Flask's teardown only cleaned the
  request-scoped `g.db_session` while a separate `_thread_engines` dict
  accumulated NullPool engines per thread, leaking ~3 FDs per request.
  Added `cleanup_current_thread()` across six tier-1 paths.
- **#2591** — dead-thread engines (when threads crashed they left
  engines behind) plus `stream=True` socket holds in the generic
  downloader. Added a throttled dead-thread sweep, removed `stream=True`,
  raised the Docker ulimit from 1024 to 65536.

### Wave 3 — LLM wrapper lifecycle (Mar 2026)

- **#2708** — diagnosed `ChatOllama` → `httpx.Client` chains with no
  `__del__`. With the news scheduler triggering 50–300
  `quick_summary()` calls per hour, a 1024-FD container exhausted in
  3–4 hours. Wrapped four programmatic API entry points in
  `try/finally` with explicit close.
- **#2712** — extracted `close_llm()` to a shared utility. Added
  `close()` and `_owns_llm` to `NewsAnalyzer`, `HeadlineGenerator`,
  `TopicGenerator`, `JournalReputationFilter`, `DomainClassifier`,
  `GitHubSearchEngine`, `IntegratedReportGenerator`,
  `ElasticsearchSearchEngine`, and the benchmark graders.
- **#2756** — wrapped bare `.close()` calls in `finally` blocks with
  `safe_close()` to prevent masking the original exception.
- **#2732** — moved `close()` into `ProcessingLLMWrapper` and
  `RateLimitedLLMWrapper` directly; eliminated the standalone
  `close_llm()` free function.

### Wave 4 — DB session leaks + per-call patterns (late Mar / early Apr 2026)

- **#3018** — `get_settings_manager(username=...)` was bypassing
  `g.db_session` and creating QueuePool sessions per-thread; live
  diagnostics showed 321 sockets allocated, only 66 in use.
  `DownloadService.close()` leaked the inner `SettingsManager` session.
  Also fixed `TopicBasedRecommender._create_recommendation_card()`
  (per-call LLM with no cleanup) and an `asyncio.new_event_loop()` in
  `news_strategy.py` that never closed.
- **#3204** — test fixtures using `return` instead of `yield` left
  engines un-disposed. Migrated 8 test files to `yield` +
  `engine.dispose()`.

### Wave 5 — DB pool architecture (Apr 2026)

- **#3340** — kept QueuePool but minimized FDs (`pool_size=1`,
  `max_overflow=2`, periodic `dispose()` every 30 min).
- **#3337** (closed) — proposed switching SQLCipher engines to
  NullPool for zero persistent FDs. Superseded by #3441.
- **#3441** — removed per-thread NullPool engines entirely
  (~2,100 lines of sweep logic deleted) and routed metrics through a
  single shared per-user QueuePool with bounded sizing
  (`pool_size=20`, `max_overflow=40`).
- **#3477** — created [ADR-0004](../decisions/0004-nullpool-for-sqlcipher.md)
  capturing the final pool model and updated stale FD calculations
  across docs.

### Wave 6 — async client close (May 2026)

- **#3818** (open, declined for merge) — proposed session-pooling
  around `safe_get`/`safe_post` to address #3816. The session refactor
  is reasonable in isolation, but the lsof in #3816 showed ~72% of
  leaked FDs as `a_inode [eventpoll]` selectors, not HTTP request
  sockets — pointing at async-client transports rather than `safe_get`
  callers (whose response bodies were already consumed). See
  [the PR comment](https://github.com/LearningCircuit/local-deep-research/pull/3818#issuecomment-4402290677)
  for the full reasoning.
- **#3855** — extended `_close_base_llm()` to also close
  `ChatOllama._async_client` (the actual gap the lsof pointed to).
  Added the `IntegratedReportGenerator` close that was missing from the
  per-research `finally` block. Idempotency via `_ldr_closed` sentinels
  on the inner httpx clients.

---

## Intentionally not done (deferred)

These showed up during planning and were deliberately *not* done. If
they get rediscovered as "missing work" by future contributors, please
reference this section first.

- **`weakref.finalize` defense-in-depth on the LLM wrappers.** Designed
  and verified safe (no `__del__` conflicts, `__getattr__` doesn't
  intercept `_finalizer`, no reference cycles). Deferred until a
  fourth wave of "missed close" leaks justifies adding a new pattern
  that future contributors must understand. Current explicit-close
  discipline has held since #2712 / #2732 / #3018.
- **LLM caching in `get_llm()`.** Bounding total `ChatOllama` instances
  to N=distinct configs would make leak shapes architecturally
  impossible. Orthogonal optimization, deferred — adds complexity
  around settings invalidation and multi-tenant isolation.
- **Pre-commit hook flagging `get_llm()` callers without `close()`.**
  Useful in principle, deferred — high false-positive risk
  (caller-passed LLMs, lazy-init holders, factory-returned LLMs all
  legitimately don't close). Needs a careful design.
- **Dedicated `/api/v1/health/fd` diagnostic with eventpoll-inode
  dedupe.** PR #3033 stalled at a basic version (Windows + RLIM_INFINITY
  bugs); PR #3036 added `utilities/fd_monitor.py` for cross-platform FD
  reading. A type/inode-breakdown extension is feasible but deferred
  until an active leak hunt actually needs it.

---

## Glossary

- **`_owns_llm`** — instance flag set in `__init__` to `True` when the
  class fetched its own LLM via `get_llm()`, `False` when an LLM was
  injected by the caller. Gates whether `close()` actually closes the
  LLM.
- **`safe_close(resource, name)`** — helper in `utilities/resource_utils.py`
  that calls `resource.close()` inside a try/except, logging on failure.
  Never raises. Used in every `finally` block.
- **`_ldr_closed`** — sentinel attribute set on inner httpx clients by
  `_close_base_llm` to make the function idempotent. Checked with
  `is True` (not truthy) so Mock objects without a `spec` don't trip
  the guard.
- **eventpoll FD** — Linux `a_inode` file descriptor type for
  `epoll_create`'d kernel objects. Each asyncio event loop registers
  one. Leaked AsyncClients hold them via the loop's selector.
