Rust: Tracking issue for the OOM hook

[glandium]

PR #50880 added an API to override the std OOM handler, similarly to the panic hook. This was discussed previously in issue #49668, after PR #50144 moved OOM handling out of the Alloc/GlobalAlloc traits. The API is somewhat similar to what existed before PR #42727 removed it without an explanation. This issue tracks the stabilization of this API.

Defined in the std::alloc module:

pub fn set_oom_hook(hook: fn(Layout) -> !);
pub fn take_oom_hook() -> fn(Layout) -> !;

pub fn set_alloc_error_hook(hook: fn(Layout));
pub fn take_alloc_error_hook() -> fn(Layout);

CC @rust-lang/libs, @SimonSapin

Unresolved questions

• [x] Name of the functions. The API before #42727 used _handler, I made it _hook in #50880 because that's the terminology used for the panic hook (OTOH, the panic hook returns, contrary to the OOM hook). #51264
• [ ] Should this move to its own module, or stay in std::alloc?
• [ ] Interaction with unwinding. alloc::alloc::oom is marked #[rustc_allocator_nounwind], so theoretically, the hook shouldn't panic (except when panic=abort). Yet if the hook does panic, unwinding seems to happen properly except it doesn't.

[SimonSapin]

We have an accepted RFC https://github.com/rust-lang/rust/issues/48043 that includes adding a way to opt into of panicking instead of aborting on OOM (so that catch_unwind could be used to recover from OOM at coarse granularity). Perhaps this could be the mechanism for that?

[glandium]

Oh right, I had forgotten about that. And in fact, it works:

#![feature(allocator_api)]

use std::alloc::set_oom_hook;
use std::alloc::Layout;

fn oom(layout: Layout) -> ! {
    panic!("oom {}", layout.size());
}

fn main() {
    set_oom_hook(oom);
    let result = std::panic::catch_unwind(|| {
        let v = Vec::<u8>::with_capacity(100000000000000);
        println!("{:p}", &v[..]);
    });
    println!("{:?}", result);
}

cargo run with last nightly:

thread 'main' panicked at 'oom 100000000000000', src/main.rs:7:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.
Err(Any)

[glandium]

(despite the #[rustc_allocator_nounwind] annotation on alloc::alloc::oom)

[glandium]

Added item about interaction with unwinding.

[nikic]

@glandium While this example works, the #[rustc_allocator_nounwind] annotation will (likely) prevent some drops from running during unwinding (in particular those that would have to be run inside the function that performs the oom call).

[glandium]

Picture me puzzled.

#![feature(allocator_api)]

use std::alloc::set_oom_hook;
use std::alloc::Layout;

struct Foo;

impl Drop for Foo {
    fn drop(&mut self) {
        println!("Foo");
    }
}

fn oom(layout: Layout) -> ! {
    let f = Foo;
    panic!("oom {}", layout.size());
}

fn main() {
    set_oom_hook(oom);
    let result = std::panic::catch_unwind(|| {
        let f = Foo;
        let v = Vec::<u8>::with_capacity(100000000000000);
        println!("{:p}", &v[..]);
    });
    println!("{:?}", result);
}

thread 'main' panicked at 'oom 100000000000000', src/main.rs:16:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.
Foo
Foo
Err(Any)

[glandium]

Or

#![feature(allocator_api)]

use std::alloc::set_oom_hook;
use std::alloc::Layout;

struct Foo;

impl Drop for Foo {
    fn drop(&mut self) {
        println!("Foo");
    }
}

fn oom(layout: Layout) -> ! {
    let f = Foo;
    panic!("oom {}", layout.size());
}

fn main() {
    set_oom_hook(oom);
    let result = std::panic::catch_unwind(|| {
        let f = Foo;
        std::alloc::oom(Layout::new::<u8>());
        println!("after oom");
    });
    println!("{:?}", result);
}

thread 'main' panicked at 'oom 1', src/main.rs:16:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.
Foo
Foo
Err(Any)

[glandium]

Ah, I forgot --release, and with it, in both cases, the Foo instance in the closure is not dropped (the one in oom is, though).

[alexcrichton]

With panic hooks they notably return () instead of ! which allows you to chain them (if necessary). I think if we want to use the terminology "hook" for OOM we probably want the same (not returning !) as that'll allow us to also justify take_oom_hook as otherwise I think you can't actually call that and/or delegate in an order where you don't go first?

OOM hooks could always panic/abort themselves of course but the signature may just want to be that by default we don't require it and then the fallback of OOM is to abort as OOM does today

[glandium]

take_oom_hook is still useful albeit annoying, since hooks can't be closures. So you'd have to store the value in a global, but then you can call it from your own function. I can see how returning () would make things more flexible, though.

[SimonSapin]

Why does take_ unregister the hook, as well as returning it? More generally, what’s an example where this function is useful?

[glandium]

This matches panic hooks, and while not exactly simple (because you can't use a closure as hook), it can be used to do whatever your hook does, while still printing the default message from libstd, whatever it is, without having to care about doing it properly yourself (which the current implementation doesn't do, since it can end up allocating aiui, which rather makes the point: one shouldn't try to replicate the code of the default handler)

[glandium]

Or, if another hook was already set, it allows to fall back to that one.

[SimonSapin]

On accessing the current/default hook, sure. But why unregistering it in the process? fn(…) pointer types are Copy, unlike Box<Fn(…)>.

[glandium]

Would you rather add a function to unregister the current hook and break the similarity with the panic hook?

[sfackler]

Why is the oom hook a function rather than a closure?

[glandium]

Because a closure can't be stored in an Atomic afaict, and using an Atomic is necessary because RwLock or Mutex can be lazily initialized and allocate memory on some platforms. You don't want to be allocating memory when trying to retrieve the hook while you're handling an oom.

[sfackler]

Seems like we can avoid that problem with a flag tracking if set_hook has ever been called. If it hasn't, we just call the default hook directly and avoid touching the mutex.

[glandium]

Well, another (new) reason is that we eventually want to move this to liballoc, which doesn't have access to any kind of locking primitive.

[gnzlbg]

@glandium

Because a closure can't be stored in an Atomic afaict,

It can (playground):

#![feature(atomic_integers, const_fn)]
use std::sync::atomic::AtomicUsize;
use std::sync::atomic::Ordering;

const unsafe fn transmute(x: *mut u8) -> usize {
    union T {
        a: *mut u8,
        b: usize
    }
    T { a: x }.b
}

const BAR: *mut u8 = ((|| 3) as fn() -> i32) as *mut u8;
const FOO: AtomicUsize = AtomicUsize::new(unsafe { transmute(BAR) });
// static FOO: AtomicUsize = AtomicUsize::new(unsafe { transmute(BAR) }); // ALSO OK

fn main() {
    let l = FOO.load(Ordering::Relaxed);
    let l: fn() -> i32 = unsafe { std::mem::transmute(l) };
    assert_eq!(l(), 3);
}

[SimonSapin]

Only closures that do not capture anything can be cast to function pointers, not arbitrary F: Fn(…) values.

[SimonSapin]

https://github.com/rust-lang/rust/pull/51543 propose renaming the functions to set_alloc_error_hook and take_alloc_error_hook.

[gnzlbg]

Only closures that do not capture anything can be cast to function pointers, not arbitrary F: Fn(…) values.

Of course. I thought we were only talking about closures without any environment, since I don't see how panic hooks with state make much sense (maybe someone can elaborate on that).

If you need to set a closure with an environment as a panic hook you just have to implement a struct representing the environment, implement the fn traits on that, and put the struct in a static in a properly synchronized way. Depending on how big the environment is, you might get away with a single extra atomic, or you might need a Mutex+Arc combo for it. Using a Mutex inside a panic hook sounds like a pretty bad idea to me though.

[sfackler]

@gnzlbg the simple case is grabbing the old hook and wrapping it with your own.

[landaire]

Just wanted to provide an update to @glandium's sample code that works as of March 2020. Some of the PRs changed the feature name this was gated behind and renamed the function to set the error hook.

#![feature(alloc_error_hook)]

use std::alloc::set_alloc_error_hook;
use std::alloc::Layout;

fn oom(layout: Layout) {
    panic!("oom {}", layout.size());
}

fn main() {
    set_alloc_error_hook(oom);
}

[mitsuhiko]

About the open point 2 "Interaction with unwinding". It would be really nice if this could allow the hook to panic. Reason being that you can then catch such a panic at the C boundary. This would help cases like this: https://github.com/hyperium/hyper/issues/2265

[seanmonstar]

Besides the interaction with unwinding, which I would like to be able to rely on, I'm also interested in the timeline of stabilizing this. It looks fairly quiet, if we answer the unwinding question, is there anything else preventing stabilizing soon?