Skip to content

fix(rust/ffi)!: require Send for exported types - #4874

Merged
lidavidm merged 1 commit into
apache:mainfrom
lidavidm:rs-export-send
Oct 9, 2026
Merged

lidavidm merged 1 commit into
apache:mainfrom
lidavidm:rs-export-send

Conversation

@lidavidm

@lidavidm lidavidm commented Oct 8, 2026

Copy link
Copy Markdown
Member

This matches the specification in the C header: handles aren't necessarily usable concurrently, but they can be used from different threads so long as access does not overlap.

Assisted-by: GPT-6.1 Sol

This matches the specification in the C header: handles aren't
necessarily usable concurrently, but they can be used from
different threads so long as access does not overlap.

Assisted-by: GPT-6.1 Sol

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The bounds correctly enforce serialized cross-thread handle movement without requiring concurrent access.

0 open findings

What changed in this PR

Requires Rust-exported ADBC handles to be safely movable between threads, matching the C API contract.

Changes:

  • Adds Send bounds for database, connection, and statement types.
  • Documents the new export requirement.
File Description
rust/​ffi/​src/​driver_exporter.rs Enforces and documents Send for exported handle types.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

@lidavidm
lidavidm marked this pull request as ready for review October 8, 2026 11:51
@lidavidm
lidavidm requested a review from wjones127 as a code owner October 8, 2026 11:51
@lidavidm
lidavidm requested a review from eitsupi October 8, 2026 23:42
@lidavidm
lidavidm merged commit 9cd246e into apache:main Oct 9, 2026
24 checks passed
@lidavidm
lidavidm deleted the rs-export-send branch October 9, 2026 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants