fix(go): fix potential deadlocks in reader #60
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What's Changed
Fix Critical Deadlocks and Race Conditions in Snowflake Record Reader
This PR addresses multiple critical concurrency issues in the Snowflake driver's
recordReaderthat could cause complete application hangs under normal racing conditions.Issues Fixed
1. Critical Deadlock:
Release()Blocking ForeverProblem: When
Release()was called while producer goroutines were blocked on channel sends, a permanent deadlock occurred:Release()cancels context and attempts to drain channelsch <- reccannot see the cancellationRelease()blocks forever onfor rec := range chFix: Added a
donechannel that signals when all producer goroutines have completed.Release()now waits for this signal before attempting to drain channels.2. Severe Deadlock: Non-Context-Aware Channel Sends
Problem: Channel send operations at lines 694 and 732 checked context before the send but not during:
Fix: Wrapped all channel sends in
selectstatements with context awareness:3. Critical Race Condition: Nil Channel Reads
Problem: Channels were created asynchronously in goroutines after
newRecordReaderreturned. IfNext()was called quickly after creation, it could read from uninitialized (nil) channels, causing infinite blocking.Fix: Initialize all channels upfront before starting any goroutines:
4. Goroutine Leaks on Initialization Errors
Problem: Error paths only cleaned up the first channel, potentially leaking goroutines if initialization failed after starting concurrent operations.
Fix: Moved all error-prone initialization (GetStream, NewReader) before goroutine creation, and added proper cleanup on errors.
Changes
donechannel toreaderstruct to signal goroutine completionselectstatements for all channel operationsRelease()to wait ondonechannel before drainingdonechannel after all producers finishReproduction Scenarios Prevented
Deadlock:
Race Condition:
Backport of apache/arrow-adbc#3870.