toddmeng-db commented on code in PR #3217:
URL: https://github.com/apache/arrow-adbc/pull/3217#discussion_r2241477161
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2Statement.cs:
##########
@@ -394,6 +404,10 @@ public override void Dispose()
HiveServer2Connection.HandleThriftResponse(resp.Status,
activity);
OperationHandle = null;
}
+ // For DatabricksReader, must be called after CloseOperation,
otherwise it may cancel a
+ // request when the input buffer of Thrift client has not been
consumed/cleaned-up
+ _currentReader?.Dispose();
+ _currentReader = null;
Review Comment:
Encountered during testing:
When currentReader.Dispose() is called before CloseOperation in this Dispose
method, the Thrift Client may have received a full or partial
GetOperationStatusResp in the input-buffer. However, if cancellation occurs
before the method recv_GetOperationStatus completes, it would leave
stranded/unconsumed response data in the input buffer.
Changing to the ordering above, it means that CloseOperation happens
strictly after GetOperationStatus (which is guarded by a lock on the
ThreadSafeClient) call completes. No cancellation can interrupt
recv_GetOperationStatus until after CloseOperation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
