2

I've been playing around with writing Redis Modules in Rust. This is my first attempt at using Rust FFI and bindings. How do I call this method and end up with a data value in Rust without destroying the Redis pointer?

extern "C" {
    pub static mut RedisModule_GetTimerInfo: ::std::option::Option<
        unsafe extern "C" fn(
            ctx: *mut RedisModuleCtx,
            id: RedisModuleTimerID,
            remaining: *mut u64,
            data: *mut *mut ::std::os::raw::c_void,
        ) -> ::std::os::raw::c_int,
    >;
}

See the RedisModule_GetTimerInfo API Docs for more details.

I ended up getting this to work, but it throws an error if I call it with the same id twice: 

let mut ms = 0 as u64;
let val = "";
let ptr = Box::into_raw(Box::new(&mut val)) as *mut *mut c_void;
let ok = unsafe { RedisModule_GetTimerInfo.unwrap()(ctx, id, &mut ms, ptr) };
let mut data: Option<String> = None;
if ok == 0 {
    let val = unsafe { Box::from_raw(*ptr as *mut &str) };
    // trim nul bytes
    data = Some(val.trim_matches(char::from(0)).to_string());
}

This didn't work because of how Box::from_raw owns the raw pointer and the pointer is destroyed when the box is dropped.

I tried countless ways to make this work without using Box::into_raw & Box::from_raw and all of times they either end up crashing Redis or end up as a pointer that I don't know how to convert to &str.

Update: I originally had an example of using RedisModule_StopTimer which was a mistake. Corrected to use the method I was asking about.

Greg Melton
  • 81
  • 7
  • `&str` is a fat pointer, which can never fit in a `*mut c_void`. You need to add more indirection, as shown by [How do I pass a closure through raw pointers as an argument to a C function?](https://stackoverflow.com/q/38995701/155423); [How do I convert a Rust closure to a C-style callback?](https://stackoverflow.com/q/32270030/155423); [Segmentation fault when using C callback user data to store a boxed Rust closure](https://stackoverflow.com/q/50955611/155423). – Shepmaster Apr 01 '20 at 13:24
  • Also related [How do I convert a C string into a Rust string and back via FFI?](https://stackoverflow.com/q/24145823/155423) – Shepmaster Apr 01 '20 at 13:25
  • I wonder how you end up with `data: *mut *mut ::std::os::raw::c_void,` in C. It's look like bad C code – Stargateur Apr 01 '20 at 14:01
  • It's not clear how you are trying to use this API. From skimming the docs, it looks like you need to pass a raw pointer to `RedisModule_CreateTimer`, and when you call `RedisModule_CreateTimerInfo` with a non-null `data` pointer it will copy the original into the pointee. So... what value did you use to *create* the timer? And what's the connection between the function you linked to and the code example in the question? (Did you link to the wrong one by accident?) – trent Apr 01 '20 at 17:15
  • @trentcl Sorry, yes. You're correct. I've corrected the example. The `RedisModule_StopTimer` does something similar using the `*mut *mut c_void` to read the data back out. – Greg Melton Apr 01 '20 at 21:41
  • Got that part, thanks for the update -- but I still think the problem is underspecified. You can't pull anything out of a `void *` unless you know what type it *should* be (the thing that was originally casted to `void *` to pass to `RedisModule_CreateTimer`). What type was that? – trent Apr 01 '20 at 21:48

2 Answers2

3

I'm one of the maintainers of the redismodule-rs crate, which provides a high-level Rust API for writing Redis modules.

Prompted by your question, I looked into adding these timer APIs to the crate in a safe manner, and will push the code to the repo once I'm done with it.

The following code shows how to retrieve the data safely:

// Create local variables to hold the returned values
let mut remaining: u64 = 0;
let mut data: *mut c_void = std::ptr::null_mut();

// Call the API and retrieve the values into the local variables
let status = unsafe {
    RedisModule_GetTimerInfo.unwrap()(ctx, timer_id, &mut remaining, &mut data)
};

if status == REDISMODULE_OK {
    // Cast the *mut c_void supplied by the Redis API to 
    // a raw pointer of our custom type:
    let data = data as *mut T; // T is the type of the data, e.g. String

    // Dereference the raw pointer (we know this is safe,
    // since Redis should return our original pointer which
    // we know to be good), and turn in into a safe reference:
    let data = unsafe { &*data };

    println!("Remaining: {}, data: {:?}", remaining, data);
}
gavrie
  • 1,641
  • 1
  • 15
  • 14
  • `Box::from_raw` + `Box::leak` seems a bit circuitous when you already have a `*mut String`; any reason not to simply dereference? `let data: &String = &*(data as *mut _);` – trent Apr 02 '20 at 16:05
  • That's a good point. I believe writing it this way makes it obvious to the reader of the code that the data originated as a `Box`. I'd also guess that the compiled code would be pretty similar. – gavrie Apr 02 '20 at 16:37
  • Following up on this, I thought about it some more and am thinking of changing my code to use regular references instead of `Box`es, which are just a special case. I'll see how it works out. – gavrie Apr 02 '20 at 20:14
  • It occurs to me that dereferencing also has an advantage over `Box::from_raw` in that you can create multiple shared references from the same `*mut String`, whereas it's unsound for a `Box` to be aliased, so you would have to be careful never to call `Box::from_raw` again while any `data`s are still hanging around. (Of course this only applies if you don't need `&mut` access to the `String`.) – trent Apr 02 '20 at 20:54
  • Yes indeed, that can be a significant advantage. – gavrie Apr 04 '20 at 09:48
  • I've updated the example to remove the superfluous `Box`-related code. – gavrie Apr 05 '20 at 18:12
1

Using one of the links @Shepmaster added, I was finally able to figure this out. I swear I tried some variation of this but didn't think to try double boxing...

Here's what I did:

    let val = Box::new(Box::new("") as Box<&str>);
    let ptr = Box::into_raw(val);
    let ok = unsafe { RedisModule_GetTimerInfo.unwrap()(ctx, id, &mut ms, ptr as *mut *mut c_void) };
    let mut data: Option<String> = None;
    if ok == 0 {
        let val = unsafe {**ptr as &str};
        data = Some(val.trim_matches(char::from(0)).to_string());
    }

Thanks all for your help!

Greg Melton
  • 81
  • 7