Skip to content

Conversation

@simonw
Copy link
Owner

@simonw simonw commented Dec 13, 2025

From 409 warnings down to 52 warnings.

Claude Code says: https://gistpreview.github.io/?69d2ce58f09fdd8463e384cc1ebee4cf

Fixed connection leaks in:

  1. datasette/utils/sqlite.py - _sqlite_version() now closes connection
  2. datasette/cli.py - --create flag now closes connection
  3. datasette/app.py - _versions() now closes connection
  4. datasette/utils/init.py - detect_json1() now closes connection when created internally
  5. tests/conftest.py - pytest_report_header() now closes connection
  6. tests/utils.py - has_load_extension() now closes connection
  7. tests/fixtures.py - app_client_no_files and CLI fixtures now close connections
  8. tests/test_api_write.py - ds_write fixture closes both connections
  9. tests/test_cli.py - Multiple test functions now close connections
  10. tests/test_config_dir.py - config_dir and config_dir_client fixtures now close connections
  11. tests/test_crossdb.py - Loop connections now closed
  12. tests/test_internals_database.py - Test setup connections now closed
  13. tests/test_plugins.py - view_names_client fixture and test now close connections
  14. tests/test_utils.py - Multiple test functions now close connections

Refs:


📚 Documentation preview 📚: https://datasette--2615.org.readthedocs.build/en/2615/

From 409 warnings down to 52 warnings.

Claude Code says:

Fixed connection leaks in:
1. datasette/utils/sqlite.py - _sqlite_version() now closes connection
2. datasette/cli.py - --create flag now closes connection
3. datasette/app.py - _versions() now closes connection
4. datasette/utils/__init__.py - detect_json1() now closes connection when created internally
5. tests/conftest.py - pytest_report_header() now closes connection
6. tests/utils.py - has_load_extension() now closes connection
7. tests/fixtures.py - app_client_no_files and CLI fixtures now close connections
8. tests/test_api_write.py - ds_write fixture closes both connections
9. tests/test_cli.py - Multiple test functions now close connections
10. tests/test_config_dir.py - config_dir and config_dir_client fixtures now close connections
11. tests/test_crossdb.py - Loop connections now closed
12. tests/test_internals_database.py - Test setup connections now closed
13. tests/test_plugins.py - view_names_client fixture and test now close connections
14. tests/test_utils.py - Multiple test functions now close connections

Refs #2614
@simonw simonw added the tests label Dec 13, 2025
@codecov
Copy link

codecov bot commented Dec 13, 2025

Codecov Report

❌ Patch coverage is 79.51807% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.05%. Comparing base (3b4c7e1) to head (8895c4a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
datasette/database.py 76.05% 17 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2615      +/-   ##
==========================================
- Coverage   90.20%   90.05%   -0.16%     
==========================================
  Files          51       51              
  Lines        7497     7571      +74     
==========================================
+ Hits         6763     6818      +55     
- Misses        734      753      +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@simonw
Copy link
Owner Author

simonw commented Dec 13, 2025

Got Claude Code to fix the remaining warnings! The terminal to HTML output is a huge 4MB, recorded here: https://static.simonwillison.net/static/2025/big-terminal.html

The summary:

The fix involved adding proper connection cleanup in datasette/database.py:

  1. Added __del__ method (lines 73-85): Ensures connections are closed when the Database object is garbage collected, preventing ResourceWarning about unclosed connections even if close() isn't called explicitly.
  2. Enhanced close() method (lines 135-200):
  • Closes thread-local connections in the main thread before removing references
  • Uses a barrier-based approach to submit cleanup tasks to ALL executor threads, ensuring thread-local connections are closed before being garbage collected
  • Closes all tracked connections in _all_connections
  1. Added stale connection check in execute_fn() (lines 349-353): Clears cached thread-local connections if the database has been closed, preventing reuse of closed connections.

Additionally, cleanup code was added/updated in test fixtures:

  • tests/fixtures.py: make_app_client() closes databases before shutting down the executor
  • tests/conftest.py: pytest_unconfigure hook cleans up the global _ds_instance
  • tests/test_permissions.py: Individual tests that create Datasette instances have proper cleanup in finally blocks
  • tests/test_docs_plugins.py: Fixture properly cleans up the Datasette instance

The key insight was that connections stored in thread-local storage (threading.local()) were being garbage collected without being explicitly closed, which triggered ResourceWarning. By adding del and ensuring cleanup happens in the correct order (databases closed before executor shutdown), all warnings are now eliminated.

@simonw
Copy link
Owner Author

simonw commented Dec 13, 2025

Gonna have to review this very carefully, there's a lot of deep clever stuff going on.

Comment on lines 673 to +685
def detect_json1(conn=None):
close_conn = False
if conn is None:
conn = sqlite3.connect(":memory:")
close_conn = True
try:
conn.execute("SELECT json('{}')")
return True
except Exception:
return False
finally:
if close_conn:
conn.close()
Copy link
Owner Author

Choose a reason for hiding this comment

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

Looks good.

Comment on lines +23 to +32
conn = sqlite3.connect(":memory:")
try:
return tuple(
map(
int,
conn.execute("select sqlite_version()").fetchone()[0].split("."),
)
)
)
finally:
conn.close()
Copy link
Owner Author

Choose a reason for hiding this comment

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

Good pattern here too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants