rustler icon indicating copy to clipboard operation
rustler copied to clipboard

Exposing enif_schedule_nif safely

Open jorendorff opened this issue 8 years ago • 5 comments

enif_schedule_nif comes with a contract, much like enif_raise_exception. It's hard to support safely.

But we already support enif_raise_exception (see https://github.com/hansihe/rustler/blob/master/src/codegen_runtime.rs#L44 ). We could support enif_schedule_nif in the same way. Don't let the user call it directly, but let them return a special Rust value that means "call enif_schedule_nif safely for me".

There is one problem that we would need to solve, though. Currently, all NIFs have the return type NifResult<NifTerm<'a>>, allowing just two possibilities:

  • Ok(term): "return this term"
  • Err(exc): "raise this exception"

Now we want to add a third option, "schedule this nif". But we can't just change the return type, because that would break all existing code. I guess the way to do this is with a trait. Come to think of it, we should allow a wide variety of return types anyway, not just NifResult<NifTerm<'a>>; for example, anything that implements NifEncode should be just as good as a NifTerm, right?

jorendorff avatar Jul 12 '17 16:07 jorendorff

Would it really be a bad thing to change the return type to add a Sched(...) option? It obviously would break existing code trying to upgrade, but Rustler isn't at the magical "1.0" yet so there is a little more flexibility.

treyzania avatar Jul 18 '17 18:07 treyzania

I think using a trait is better. The idea is that the user decides what the return type is, and Rustler figures out what to do with it.

So a user may keep using NifResult<NifTerm> for their existing NIFs... and write a new NIF that returns NifResult<NifSched> ... and write another new NIF that returns a Rust String, there's no reason Rustler couldn't handle all of them.

jorendorff avatar Jul 20 '17 00:07 jorendorff

I don't know how readable this is, but here's the approach I took for a little Scheme implementation I've been messing around with: https://github.com/jorendorff/cell-gc/commit/149ce249bb4ac565783deeef9bcdbc502d061e5d

Before:

fn char_upcase<'h>(_hs: &mut GcHeapSession<'h>, args: Vec<Value<'h>>) -> Result<Trampoline<'h>> {
    if args.len() != 1 {
        return Err("char-upcase: 1 argument required".into());
    }
    let c = args[0].clone().as_char("char-upcase: character required")?;

    // I think the only character that doesn't upcase to a single character is
    // U+00DF LATIN SMALL LETTER SHARP S ('ß'). Guile returns it unchanged.
    // Fine with me.
    let mut up = c.to_uppercase();
    let result = match (up.next(), up.next()) {
        (Some(d), None) => d,
        _ => c,
    };

    Ok(Trampoline::Value(Value::Char(result)))
}

After:

builtins! {
    fn char_upcase "char-upcase" <'h>(_hs, c: char) -> char {
        // I think the only character that doesn't upcase to a single character is
        // U+00DF LATIN SMALL LETTER SHARP S ('ß'). Guile returns it unchanged.
        // Fine with me.
        let mut up = c.to_uppercase();
        match (up.next(), up.next()) {
            (Some(d), None) => d,
            _ => c,
        }
    }
}

This probably looks like it's solving a completely different problem. But if we can cope with return values like char, we can also cope with NifResult<NifTerm> vs. NifResult<NifSched> or whatever.

jorendorff avatar Aug 01 '17 21:08 jorendorff

Of course I don't mean to propose any particular detail of that for Rustler. In particular, that builtins! macro syntax is silly. 🤡

I just wanted to show that something like this is possible. And the traits I used aren't much different from the NifEncode and NifDecode traits Rustler already has.

jorendorff avatar Aug 01 '17 22:08 jorendorff

Requesting comments on an updated PR for this enhancement here: https://github.com/rusterlium/rustler/pull/232

sunny-g avatar May 26 '21 22:05 sunny-g