Skip to content

Mark OnceState::poison as pub #509

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

Closed
GrigorenkoPV opened this issue Dec 19, 2024 · 2 comments
Closed

Mark OnceState::poison as pub #509

GrigorenkoPV opened this issue Dec 19, 2024 · 2 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@GrigorenkoPV
Copy link

GrigorenkoPV commented Dec 19, 2024

Proposal

To quote the tracking issue (I believe it describes the motivating use case as well as the alternatives)

The Once synchronization primitive provides a poison method for marking the internal OnceState as poisoned, which is combined with the call_once_force method in OnceLock::initialize to call initialization functions until one succeeds without panicking. This provides a way for users to use fallible initialization functions where they only observe the OnceLock being initialized once a function doesn't panic:

fn initialize<F, E>(&self, f: F) -> Result<(), E>
where
    F: FnOnce() -> Result<T, E>,
{
    let mut res: Result<(), E> = Ok(());
    let slot = &self.value;
    // Ignore poisoning from other threads
    // If another thread panics, then we'll be able to run our closure
    self.once.call_once_force(|p| {
        match f() {
            Ok(value) => {
                unsafe { (&mut *slot.get()).write(value) };
            }
            Err(e) => {
                res = Err(e);
                // Treat the underlying `Once` as poisoned since we
                // failed to initialize our value.
                p.poison();
            }
        }
    });
    res
}

This tracking issue is for marking the poison method on OnceState as pub, rather than pub(crate). This has no impact on the OnceCell or OnceLock APIs, but allows downstream libraries to build out similar functionality. The motivation for this is for the twice-cell crate, where use of this API would simplify the implementation.

Public API

// std::sync::once

pub struct OnceState {
    pub(crate) inner: sys::OnceState,
}

impl OnceState {
     /// Poison the associated [`Once`] without explicitly panicking.
    #[inline]
    pub fn poison(&self) {
        self.inner.poison();
    }
}

Links and related work

@GrigorenkoPV GrigorenkoPV added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Dec 19, 2024
@programmerjake
Copy link
Member

the link is broken, you want https://crates.io/crates/twice-cell

@dtolnay
Copy link
Member

dtolnay commented Jan 15, 2025

We discussed this proposal in today's standard library API team meeting. In the motivating example, we noticed that you are using OnceState::poison in combination with call_once_force, which is an API specifically for ignoring the poisoned state of a Once. In other words, you are not interested in poisoning the Once. You are interested in aborting the initialization and being allowed to retry it a subsequent time.

We discussed some other imaginary use cases for a OnceState::poison method revolving around usages that want to collapse the distinction between panics&errors during initialization, i.e. if initialization fails, whether because of a panic or an error, they want to poison the Once and not retry.

For your use case, methods more like "try_init" on OnceLock would be more appropriate. Those are meant for being able to abort an initialization without poisoning. See caass/twice-cell#5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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