-
Notifications
You must be signed in to change notification settings - Fork 270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(ffi): introduce AsyncRuntimeDropped
helper
#4168
Conversation
AsyncRuntimeDropped
helper
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4168 +/- ##
=======================================
Coverage 84.88% 84.88%
=======================================
Files 272 272
Lines 29133 29133
=======================================
Hits 24730 24730
Misses 4403 4403 ☔ View full report in Codecov by Sentry. |
This avoids proliferation of `ManuallyDrop` in the code base, by having a single type that's used for dropping under an async runtime.
1985150
to
a9b4867
Compare
Rebased, so ready for review. Still an open question if we want to share code between the two FFI crates, so it's possible to share this reusable thing with the crypto crate (that uses the same pattern twice). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to share the code I would say.
/// This is useful whenever such a data type may transitively call some | ||
/// runtime's `block_on` function in their `Drop` impl (since we lack async drop | ||
/// at the moment), like done in some `deadpool` drop impls. | ||
pub(crate) struct AsyncRuntimeDropped<T>(ManuallyDrop<T>); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes please!
This avoids proliferation of
ManuallyDrop
in the code base, by havinga single type that's used for dropping under an async runtime.
I actually saw three uses of
ManuallyDrop
on the FFI layer, but failed to notice that two of them were in the crypto FFI crate. Should I introduce a new FFI helper crate just for sharing this code across the two other crates?