search

apache / arrow-adbc

Database connectivity API standard and libraries for Apache Arrow
https://arrow.apache.org/adbc/
Apache License 2.0
385 stars 98 forks source link

c/driver_manager: Python bindings can segfault if `AdbcDatabaseInit()` errors #2264

Closed paleolimbot closed 1 month ago

paleolimbot commented 1 month ago

What happened?

When testing the framework driver in #2186, the driver manager segfaulted when the database init errored. The segfault happened here:

https://github.com/apache/arrow-adbc/blob/5248c3457e00770d4b5bc215e5f71013f449d1ca/c/driver_manager/adbc_driver_manager.cc#L670

...because the function pointer for the error detail was nullptr.

I think this is because the driver had been released on a call to SetOption():

https://github.com/apache/arrow-adbc/blob/5248c3457e00770d4b5bc215e5f71013f449d1ca/c/driver_manager/adbc_driver_manager.cc#L956

I am wondering if we hadn't seen this because typically database options are checked when the connection is initialized (or because drivers weren't zeroing the function pointers on release until using the framework).

An easy stopgap would be to also check that error->private_driver->ErrorGetDetail != nullptr here:

https://github.com/apache/arrow-adbc/blob/5248c3457e00770d4b5bc215e5f71013f449d1ca/c/driver_manager/adbc_driver_manager.cc#L667-L673

..or to zero out the error->private_driver after releasing the driver.

Stack Trace

No response

How can we reproduce the bug?

No response

Environment/Setup

No response

lidavidm commented 1 month ago

I think we should make both checks

This sort of thing is also why unloading drivers in the driver manager is hard/has been punted on: errors stick around and need a way to be released...