Copilot commented on code in PR #4884:
URL: https://github.com/apache/arrow-adbc/pull/4884#discussion_r4254690536
##########
csharp/src/Apache.Arrow.Adbc/C/CAdbcDriverImporter.cs:
##########
@@ -340,9 +340,16 @@ public unsafe override AdbcDatabase
Open(IReadOnlyDictionary<string, string> par
{
if (result == null && nativeDatabase.private_data != null)
{
- using (CallHelper caller = new CallHelper())
+ // Don't let a failed release hide the error that got
us here
+ try
+ {
+ using (CallHelper caller = new CallHelper())
+ {
Review Comment:
The cleanup fix only covers the string-valued `Open` overload. The adjacent
`Open(IReadOnlyDictionary<string, object>)` still calls
`DatabaseNew`/`DatabaseInit` without any `finally`; if initialization fails
after `private_data` is allocated, the native database leaks (and this PR's
cleanup-error behavior is not applied). Please use the same result/try/finally
release pattern in that overload as well.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
