Skip to content
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

Expand ThinBox API to support fallible allocation #213

Closed
tleibert opened this issue Apr 19, 2023 · 2 comments
Closed

Expand ThinBox API to support fallible allocation #213

tleibert opened this issue Apr 19, 2023 · 2 comments
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@tleibert
Copy link

Proposal

Problem statement

ThinBox can only be allocated with the standard allocator API model, where any errors encountered while allocating are handled by calling the global handler. This behavior is not desirable in environments where manual error handling is needed, even for memory allocation errors.

Motivation, use-cases

One of the motivating uses for ThinBox is passing trait objects or other DST's over an FFI boundary.
The main example I have for why we'd want this is closed-source (for now).
In my recent work with the FreeBSD kernel, I implemented the ability to use closures as callback functions/event handlers for kernel processes. However, passing them over the FFI boundary required a double-box:

let ffi_safe_box: Box<Box<dyn FnOnce(...)>> = ...;

This was required to overcome the fact that the inner box is a wide pointer, which isn't able to be safely passed to C without another layer of indirection.

ThinBox already solves this issue, but it is unusable in this project as any allocation errors will trigger a panic, and inside the kernel, a panic means the kernel itself panics and the whole system goes down.

Solution sketches

ThinBox::try_new mimics Box::try_new, and simply bubbles up any errors encountered during creation as core::alloc::AllocErrors. The function is implemented nearly identically to how the original is.

Links and related work

[Pending Open Source Release of FreeBSD kernel work]

@tleibert tleibert added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Apr 19, 2023
@the8472
Copy link
Member

the8472 commented May 10, 2023

My general "too many allocation API variants" concern applies here, elaborated in rust-lang/wg-allocators#90

@dtolnay
Copy link
Member

dtolnay commented Feb 11, 2024

Adding unstable ThinBox::try_new sounds good to me, as long as we wouldn't stabilize it before figuring out whether to keep Box::try_new which is currently unstable too.

@dtolnay dtolnay closed this as completed Feb 11, 2024
@dtolnay dtolnay added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Feb 11, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 11, 2024
Create try_new function for ThinBox

The `allocator_api` feature has proven very useful in my work in the FreeBSD kernel. I've found a few places where a `ThinBox` rust-lang#92791 would be useful, but it must be able to be fallibly allocated for it to be used in the kernel.

This PR proposes a change to add such a constructor for ThinBox.

ACP: rust-lang/libs-team#213
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 12, 2024
Rollup merge of rust-lang#110483 - tleibert:thin-box-try-new, r=dtolnay

Create try_new function for ThinBox

The `allocator_api` feature has proven very useful in my work in the FreeBSD kernel. I've found a few places where a `ThinBox` rust-lang#92791 would be useful, but it must be able to be fallibly allocated for it to be used in the kernel.

This PR proposes a change to add such a constructor for ThinBox.

ACP: rust-lang/libs-team#213
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

3 participants