feat!(rust/ffi): catch panics at FFI boundary#3819
Merged
lidavidm merged 5 commits intoapache:mainfrom Jan 6, 2026
Merged
Conversation
lidavidm
force-pushed
the
catch-panic
branch
7 times, most recently
from
e1f661e to
a2d7fc3
Compare
Member
Author
|
also CC @eitsupi if you're interested |
amoeba
approved these changes
Dec 26, 2025
Member
amoeba
left a comment
amoeba
left a comment
There was a problem hiding this comment.
Really nice. I hadn't seen the old behavior so I disabled it and re-ran the new tests and this PR is a big improvement. I had one question and a really tiny nit if you agree.
eitsupi
suggested changes
Dec 28, 2025
Contributor
eitsupi
left a comment
eitsupi
left a comment
There was a problem hiding this comment.
Great!
I found some points to be improved.
eitsupi
reviewed
Dec 28, 2025
Contributor
There was a problem hiding this comment.
Perhaps the documentation needs to be updated to state that "AdbcDriverInit is a reserved name" (no longer be able to used after this change), and perhaps the PR should be renamed to feat(rust/ffi)! ... to make it clear that it is a breaking change.
Member
Author
There was a problem hiding this comment.
Ah, I should update that comment...the name should be the latter (a driver should provide both)
lidavidm
changed the title
Member
Author
|
I think 22b954d needs a slight fix which I'll look at |
Member
Author
This gives a slightly nicer experience than unconditionally crashing the entire program. As with the Go FFI scaffolding, once a driver panics, all further calls to the driver will fail. (Albeit, this is not true for exported readers. We also need our own C Data Interface export shim so we can handle things like exporting ADBC errors through the C Data Interface boundary. Currently, C++ and Go-based drivers can do this.)
Co-authored-by: eitsupi <50911393+eitsupi@users.noreply.github.com>
Co-authored-by: Bryce Mecum <petridish@gmail.com>
Co-authored-by: Bryce Mecum <petridish@gmail.com>
lidavidm
force-pushed
the
catch-panic
branch
from
f41b35f to
e3801d0
Compare
eitsupi
approved these changes
Dec 30, 2025
jhrotko
pushed a commit
to jhrotko/arrow-adbc
that referenced
this pull request
Feb 2, 2026
This gives a slightly nicer experience than unconditionally crashing the entire program. As with the Go FFI scaffolding, once a driver panics, all further calls to the driver will fail. (Albeit, this is not true for exported readers. We also need our own C Data Interface export shim so we can handle things like exporting ADBC errors through the C Data Interface boundary. Currently, C++ and Go-based drivers can do this.) --------- Co-authored-by: eitsupi <50911393+eitsupi@users.noreply.github.com> Co-authored-by: Bryce Mecum <petridish@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This gives a slightly nicer experience than unconditionally crashing the entire program. As with the Go FFI scaffolding, once a driver panics, all further calls to the driver will fail. (Albeit, this is not true for exported readers. We also need our own C Data Interface export shim so we can handle things like exporting ADBC errors through the C Data Interface boundary. Currently, C++ and Go-based drivers can do this.)