Skip to content

fix: [#1016] release Cursor goroutines and rows when the context is canceled - #1580

Open
JGamboa wants to merge 2 commits into
goravel:masterfrom
JGamboa:fix/orm-cursor-leak
Open

JGamboa wants to merge 2 commits into
goravel:masterfrom
JGamboa:fix/orm-cursor-leak

Conversation

@JGamboa

@JGamboa JGamboa commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

📑 Description

Closes goravel/goravel#1016

orm.Query.Cursor() and db.Query.Cursor() send rows on an unbuffered channel. When the consumer stops early (a break or return), the producer goroutine blocks on the send forever, so the rows are never closed and the connection never returns to the pool.

This is a backward-compatible fix, with no signature change:

  • The send now also selects on the query context: select { case ch <- row: case <-ctx.Done(): return }. Canceling the context given to WithContext, or a request context ending, lets the goroutine exit and close the rows. Applied to both the ORM and the DB builder.
  • The Cursor doc comments in the contracts explain that the context must be canceled when stopping early.

Breaking out without canceling the context still can't be detected without changing the API. The issue lists the options, such as a new iter.Seq2-based method as a follow-up.

✅ Checks

  • Added test cases for my code
    • TestCursor_BreakWithCanceledContext in tests/query_test.go and tests/db_test.go. It fails on master and passes here on MySQL, PostgreSQL, SQL Server and SQLite.

…eled

Cursor sends rows over an unbuffered channel from a background goroutine.
If the consumer stops ranging early (break/return), the goroutine blocks
forever on the send, keeping the rows and the connection open.

The send now also selects on the query context, so canceling the context
passed to WithContext lets the goroutine exit and close the rows. Applies
to both orm.Query.Cursor and db.Query.Cursor.
@codecov

codecov Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.34%. Comparing base (7ffe4a8) to head (084beab).

Files with missing lines Patch % Lines
database/gorm/query.go 80.00% 2 Missing ⚠️
database/db/query.go 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1580      +/-   ##
==========================================
+ Coverage   72.91%   73.34%   +0.43%     
==========================================
  Files         412      412              
  Lines       26893    26909      +16     
==========================================
+ Hits        19608    19736     +128     
+ Misses       7283     7171     -112     
  Partials        2        2              

☔ View full report in Codecov by Harness.
📢 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.

@JGamboa
JGamboa marked this pull request as ready for review October 18, 2026 23:47
@JGamboa
JGamboa requested a review from a team as a code owner October 18, 2026 23:47
@JGamboa

JGamboa commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

Review Ready

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cursor() leaks a goroutine, the rows and the connection when the loop is exited early

1 participant