Skip to content

fix: close SQLite connections on exception in calculate_table_hash and _get_count#770

Merged
Scottcjn merged 1 commit intoScottcjn:mainfrom
wsimon1982:fix/sync-sqlite-connection-leak
Mar 9, 2026
Merged

fix: close SQLite connections on exception in calculate_table_hash and _get_count#770
Scottcjn merged 1 commit intoScottcjn:mainfrom
wsimon1982:fix/sync-sqlite-connection-leak

Conversation

@wsimon1982
Copy link
Copy Markdown
Contributor

Fixes #767

Problem

calculate_table_hash() and _get_count() in node/rustchain_sync.py opened a SQLite connection but relied on reaching conn.close() at the end of the function. Any exception (disk I/O error, locked DB, etc.) would bypass the close call, leaking the connection. Under sustained error conditions this exhausts the SQLite connection limit and the node can no longer open new DB connections.

Fix

Wrap all cursor operations in try/finally blocks so conn.close() is guaranteed, matching the pattern already used in apply_sync_payload and _load_table_schema.

…d _get_count

Fixes Scottcjn#767

Both calculate_table_hash() and _get_count() in RustChainSyncManager
opened a SQLite connection with _get_connection() but relied on reaching
conn.close() at the bottom of the function.  Any exception raised by
cursor.execute() or fetchall() / fetchone() would bypass conn.close(),
silently leaking the connection.

SQLite limits the number of open connections per process.  Under
sustained error conditions (e.g. disk I/O errors, locked DB) repeated
calls would exhaust that limit and make the node unable to open any new
database connections.

Wrap all cursor operations in try/finally blocks to guarantee conn.close()
is always called, mirroring the pattern already used in apply_sync_payload
and _load_table_schema.
@github-actions github-actions bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related size/S PR: 11-50 lines labels Mar 9, 2026
Copy link
Copy Markdown
Owner

@Scottcjn Scottcjn left a comment

Choose a reason for hiding this comment

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

Clean, correct fix. try/finally pattern matches existing apply_sync_payload and _load_table_schema. Prevents connection leaks under sustained error conditions.

Verdict: MERGE — 10 RTC bounty (micro fix, real bug).

🤖 Reviewed with GPT-5.4 + Claude Opus dual-brain analysis

@Scottcjn Scottcjn merged commit 2a9395f into Scottcjn:main Mar 9, 2026
3 checks passed
@Scottcjn
Copy link
Copy Markdown
Owner

Scottcjn commented Mar 9, 2026

Merged! 🎉 10 RTC bounty approved for this fix.

Please share your RustChain wallet address (or miner ID) so we can send the payment. You can create one with the wallet CLI or just pick a wallet name and we'll set it up.

Your other PRs (#769: 50 RTC, #768: 15 RTC) are also merged — total: 75 RTC. We'll batch it all in one transfer once we have your wallet.

@wsimon1982
Copy link
Copy Markdown
Contributor Author

Thanks for merging! Here is the wallet for the 75 RTC payout (batch all 3 PRs):

RustChain Wallet: wsimon1982

If a full RTC wallet address is needed, please let me know and I can generate one via the wallet CLI. Ready whenever you are!

@wsimon1982
Copy link
Copy Markdown
Contributor Author

Payout wallet for all 3 PRs (#768, #769, #770):

RTC1274aea37cc74eb889bf2abfd22fee274fc37706b

Ready to receive the 75 RTC. Thank you!

@wsimon1982
Copy link
Copy Markdown
Contributor Author

@Scottcjn — following up on the RTC payout for PRs #768, #769, #770. Wallet address confirmed:

RTC1274aea37cc74eb889bf2abfd22fee274fc37706b

Let me know if you need anything else to process the payment. Thanks!

@Scottcjn
Copy link
Copy Markdown
Owner

Scottcjn commented Mar 9, 2026

75 RTC sent to RTC1274aea37cc74eb889bf2abfd22fee274fc37706b (pending 24h confirmation, tx dc909443715678e7bd5675e63e3242ec).

Breakdown:

Thanks for the solid contributions!

@Scottcjn
Copy link
Copy Markdown
Owner

Your 75 RTC payment is already in the pending queue (pending #778). Transfers auto-confirm after 24 hours via cron. You should see it in your wsimon1982 wallet shortly.

Thanks for the solid PRs — all 3 merged.

@wsimon1982
Copy link
Copy Markdown
Contributor Author

Hi @Scottcjn,

PRs #768, #769, #770 were merged yesterday. I posted my RustChain wallet on this PR for the 75 RTC batch payout:

Wallet: RTC1274aea37cc74eb889bf2abfd22fee274fc37706b

Checked balance today — still showing 0 RTC. Is the payout pending or do I need to do anything else?

Thanks!

createkr pushed a commit to createkr/Rustchain that referenced this pull request Mar 22, 2026
…d _get_count (Scottcjn#770)

Fixes Scottcjn#767

Both calculate_table_hash() and _get_count() in RustChainSyncManager
opened a SQLite connection with _get_connection() but relied on reaching
conn.close() at the bottom of the function.  Any exception raised by
cursor.execute() or fetchall() / fetchone() would bypass conn.close(),
silently leaking the connection.

SQLite limits the number of open connections per process.  Under
sustained error conditions (e.g. disk I/O errors, locked DB) repeated
calls would exhaust that limit and make the node unable to open any new
database connections.

Wrap all cursor operations in try/finally blocks to guarantee conn.close()
is always called, mirroring the pattern already used in apply_sync_payload
and _load_table_schema.

Co-authored-by: wsimon1982 <wsimon1982@googlemail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related size/S PR: 11-50 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: SQLite connection leak in calculate_table_hash and _get_count (missing try/finally)

2 participants