wait until processImpl completes before closing stream chan

[nmerkulov]

Fix race condition in query processing

Problem

There was a race condition between closing the stream channel and writing to it from a background goroutine in conn_query.go.

Race condition scenario:

1. conn_query.go starts a goroutine that calls c.process(ctx, onProcess) (line 45)
2. c.process() spawns another goroutine internally that calls c.processImpl() (line 106 in conn_process.go)
3. c.processImpl() reads data from the server and calls on.data(block) for each data block (line 170 in conn_process.go)
4. When the context is cancelled, c.process() returns with ctx.Err()
5. The outer goroutine in conn_query.go immediately closes the stream channel (line 54)
6. However, the inner goroutine from step 2 may still be running and attempting to write to stream via on.data(block)
7. Writing to a closed channel causes a panic: panic: send on closed channel

Stack trace example:

goroutine 1: close(stream)
goroutine 2: stream <- b  // panic!

Solution

Modified conn_process.go to ensure that the process() function waits for the internal goroutine to complete before returning, even when the context is cancelled.

Changes:

• When ctx.Done() is triggered, after calling c.cancel(), we now wait for the goroutine to finish by selecting on errCh or doneCh
• This guarantees that on.data will not be called after process() returns
• The stream channel can now be safely closed in conn_query.go without race conditions

Impact

• Fixes panic when cancelling queries that are actively receiving data
• Ensures proper cleanup and resource management
• No breaking changes to public API
• Minimal performance impact (only adds synchronization on context cancellation path)

[CLAassistant]

All committers have signed the CLA.

[Copilot]

Pull Request Overview

This PR fixes a race condition in query processing where the stream channel could be closed while a background goroutine was still attempting to write to it, causing a panic.

• Adds synchronization to wait for background goroutine completion before returning from process() function when context is cancelled
• Prevents "send on closed channel" panic during query cancellation
• Ensures proper cleanup and resource management without breaking API changes

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

Corrected spelling of 'untill' to 'until' in the PR title.

[kavirajk]

@nmerkulov thanks for the PR :) do you mind adding a test case to lock the behavior?

[kavirajk]

My bit of concern with this approach is, if you look at the background goroutine, it doesn't use/watch for passed in ctx for cancellation. It does give it to upstream handle method but we cannot gurantee that handle returns as soon as passed in ctx get cancelled.

the problem is, the whole point of canceling this method is to stop doing handle and return immediately.

With this change, we may indirectly may end-up waiting for handle to "finish".

I would handle the situation ideally like following.

1. make the goroutine listen with two select case. a. handle to finish b. context to be done.
2. Based on which ever comes first (a) or (b), we exit the go-routine.

Does it make sense?

[kavirajk]

Also we may have same problem during firstBlock as well
https://github.com/nmerkulov/clickhouse-go/blob/24bbc84a5899c097a66469e063b3ddb3eba39045/conn_process.go#L30-L46