Skip to content

fix: close rows after query timeout to prevent hang#702

Open
dlevy-msft-sql wants to merge 1 commit intomainfrom
fix-timeout-hang
Open

fix: close rows after query timeout to prevent hang#702
dlevy-msft-sql wants to merge 1 commit intomainfrom
fix-timeout-hang

Conversation

@dlevy-msft-sql
Copy link
Contributor

@dlevy-msft-sql dlevy-msft-sql commented Feb 6, 2026

Fixes sqlcmd hanging for ~10 minutes after query timeout on Linux.

Problem

When using the -t (query timeout) flag, sqlcmd correctly prints "Timeout expired" but then hangs for approximately 10 minutes before exiting. This occurs because the rows result set from QueryContext is never closed when a timeout occurs.

Reproduction

# This should timeout after 1 second, but hangs for ~10 minutes
sqlcmd -S server -t 1 -Q "WAITFOR DELAY '00:00:10'"

Root Cause

In pkg/sqlcmd/sqlcmd.go, the runQuery function calls db.QueryContext() but doesn't close the returned rows object. When the context times out, the query is cancelled but the rows handle remains open, causing the underlying connection to wait for the driver's internal cleanup timeout.

Fix

Add defer rows.Close() immediately after QueryContext returns:

rows, qe := s.db.QueryContext(ctx, query, retmsg)
if rows != nil {
    defer func() { _ = rows.Close() }()
}

This ensures the result set is always closed, even when the context is cancelled due to timeout.

Changes

File Change
pkg/sqlcmd/sqlcmd.go Add defer rows.Close() after QueryContext
cmd/modern/e2e_test.go Add TestE2E_QueryTimeout_NoHang regression test

Testing

The new test verifies that a 1-second timeout query completes within 30 seconds (not 10 minutes):

func TestE2E_QueryTimeout_NoHang(t *testing.T) {
    // Uses -t 1 with WAITFOR DELAY '00:00:10'
    // Asserts command completes in < 30 seconds
}

Related

Replaces #682 (closed due to lint error and code quality issues).

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.

2 participants