Skip to content

Explicitly close sqlite connections in ChannelIndex#287

Merged
dholth merged 5 commits into
mainfrom
fix/close-sqlite-connections-236
May 4, 2026
Merged

Explicitly close sqlite connections in ChannelIndex#287
dholth merged 5 commits into
mainfrom
fix/close-sqlite-connections-236

Conversation

@jezdez
Copy link
Copy Markdown
Member

@jezdez jezdez commented Apr 29, 2026

Description

ChannelIndex.cache_for_subdir() returns a CondaIndexCache whose db attribute lazily opens an sqlite3.Connection on first access. Five callers in ChannelIndex (extract_wrapper, index_subdir, index_subdir_shards, _update_channeldata, build_run_exports_data) drop the cache reference without ever calling close(), so the connections stay open until the GC finalizes them.

This causes two issues:

  1. On Python 3.13+ the sqlite3 module emits ResourceWarning: unclosed database for every connection finalized this way. conda-build CI on Python 3.14 hits this ~1900 times per run, see for example this job.
  2. The cache .db file can stay open longer than API consumers expect after update_index() returns, which is the original report in Use with closing(...) to clean up sqlite connections promptly #236.

This PR wraps each cache_for_subdir(subdir) call site in contextlib.closing(...), as suggested in #236.

A regression test (test_update_index_closes_sqlite_connections) subclasses sqlite3.Connection to track explicit close() calls and asserts every connection opened by update_index() is closed. The test fails on main and passes with this change.

Most of the diff line count is indentation from the new with closing(...): blocks; there are no behavior changes other than closing connections promptly.

Closes #236

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation? no documented behavior change.

@github-project-automation github-project-automation Bot moved this to 🆕 New in 🔎 Review Apr 29, 2026
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Apr 29, 2026
`ChannelIndex.cache_for_subdir()` returns a `CondaIndexCache` whose
`db` attribute lazily opens an `sqlite3.Connection` on first access.
The five callers in `ChannelIndex` (`extract_wrapper`, `index_subdir`,
`index_subdir_shards`, `_update_channeldata`, `build_run_exports_data`)
drop the cache reference without ever calling `close()`, so the
connections stay open until the GC finalizes them.

On Python 3.13+ the sqlite3 module emits `ResourceWarning: unclosed
database` for every connection finalized this way, which makes test
runs in downstream consumers like conda-build noisy and can keep the
cache `.db` file open longer than API consumers expect.

Wrap each call site in `contextlib.closing(...)` so the connection is
closed as soon as the cache is no longer needed. Add a regression test
that subclasses `sqlite3.Connection` to assert every connection opened
by `update_index()` is explicitly closed.

Closes #236
@jezdez jezdez force-pushed the fix/close-sqlite-connections-236 branch from 3a4f2a4 to 7c631ce Compare April 29, 2026 12:10
@jezdez jezdez requested a review from dholth April 29, 2026 12:31
@dholth
Copy link
Copy Markdown
Contributor

dholth commented Apr 29, 2026

Why don't we add __enter__ and __exit__ to the base class, where __exit__ calls self.close()

@jezdez
Copy link
Copy Markdown
Member Author

jezdez commented Apr 29, 2026

Why don't we add __enter__ and __exit__ to the base class, where __exit__ calls self.close()

Works for me, would be nicer, but also something we wouldn't be able to disable?

@dholth
Copy link
Copy Markdown
Contributor

dholth commented Apr 29, 2026

Do we need to be able to disable it? If you don't want it, don't use the with statement?

Comment thread tests/test_index.py Outdated
Copy link
Copy Markdown
Contributor

@dholth dholth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@github-project-automation github-project-automation Bot moved this from 🆕 New to ✅ Approved in 🔎 Review May 4, 2026
@dholth dholth merged commit a31b65d into main May 4, 2026
28 checks passed
@dholth dholth deleted the fix/close-sqlite-connections-236 branch May 4, 2026 17:37
@github-project-automation github-project-automation Bot moved this from ✅ Approved to 🏁 Done in 🔎 Review May 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed [bot] added once the contributor has signed the CLA

Projects

Status: 🏁 Done

Development

Successfully merging this pull request may close these issues.

Use with closing(...) to clean up sqlite connections promptly

3 participants