Casting c_void* for C-Callback

wspeirs · January 17, 2019, 3:29pm

I'm working on a wrapper around libhackrf: GitHub - wspeirs/rs-libhackrf: libhackrf as a Rust crate

I'm having a problem packaging up a context (generic) and function (callback), casting it as a c_void pointer, and then casting it back on the other side. The problem I have is that Valgrind indicates an uninitialized read.

The following code snipit will make more sense:

#[repr(C)]
struct CallbackContext<'a, T> {
    context: Option<&'a mut T>,
    function: &'a mut FnMut(&[u8], &Option<&mut T>) -> Error
}

impl <'a> Device<'a> {
    unsafe extern "C" fn rx_callback<C>(transfer: *mut hackrf_transfer) -> i32
    where C: std::fmt::Debug {
        let buffer :&[u8] = slice::from_raw_parts((*transfer).buffer, (*transfer).valid_length as usize);

        let ctx = (*(*transfer).device).rx_ctx;
        let ctx  = &mut *(ctx as *mut Box<CallbackContext<C>>);

        Into::into((ctx.function)(buffer, &ctx.context)) // line 88
    }

    pub fn start_rx<T, F>(&mut self, mut callback: F, rx_ctx: Option<&mut T>) -> Result<(), Error>
    where F: FnMut(&[u8], &Option<&mut T>) -> Error,
    T: std::fmt::Debug {
        unsafe {
            // package up our context and function
            let mut ctx = Box::new(CallbackContext {
                context: rx_ctx,
                function: &mut callback
            });

            // calling leak so when Box is dropped, the memory 
            let ctx = Box::leak(ctx) as *mut _ as *mut c_void;isn't freed

            // this function creates a thread (pthread) and runs it
            let ret = hackrf_start_rx(self.device_ptr, Some(Device::rx_callback::<T>), ctx);

            if ret != hackrf_error_HACKRF_SUCCESS {
                return Err(Error::from(ret));
            }
        }

        // set the state so we can cleanup properly
        self.state = State::RECEIVING;

        Ok( () )
    }
}

I'm calling start_rx as follows:

    #[test]
    fn start_stop_rx() {
        LOGGER_INIT.call_once(|| simple_logger::init_with_level(log::Level::Trace).unwrap());

        let mut hrf = HackRF::new().expect("Error creating HackRF");
        let mut dev = hrf.open_device(0).expect("Error creating device; maybe not plugged in?");

        let ctx = None::<&mut i32>;

        dev.start_rx(|b, c| {
            Error::SUCCESS
        }, ctx).expect("Error calling start_rx");

        thread::sleep_ms(250);

        dev.stop_rx().expect("Error calling stop_rx");
    }

... and the error in Valgrind:

==32509== Thread 4:
==32509== Invalid read of size 8
==32509==    at 0x135E35: rs_libhackrf::device::Device::rx_callback::haf9645800687a434 (device.rs:88)
==32509==    by 0x4E3E21C: hackrf_libusb_transfer_callback (in /usr/local/lib/libhackrf.so.0.5.0)
==32509==    by 0x5F5D159: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32509==    by 0x5F62F07: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32509==    by 0x5F5CC48: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32509==    by 0x5F5DB52: libusb_handle_events_timeout_completed (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32509==    by 0x4E3E14A: transfer_threadproc (in /usr/local/lib/libhackrf.so.0.5.0)
==32509==    by 0x54556B9: start_thread (pthread_create.c:333)
==32509==    by 0x598841C: clone (clone.S:109)
==32509==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
==32509== 
==32509== 
==32509== Process terminating with default action of signal 11 (SIGSEGV)
==32509==  Access not within mapped region at address 0x40008
==32509==    at 0x135E35: rs_libhackrf::device::Device::rx_callback::haf9645800687a434 (device.rs:88)
==32509==    by 0x4E3E21C: hackrf_libusb_transfer_callback (in /usr/local/lib/libhackrf.so.0.5.0)
==32509==    by 0x5F5D159: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32509==    by 0x5F62F07: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32509==    by 0x5F5CC48: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32509==    by 0x5F5DB52: libusb_handle_events_timeout_completed (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32509==    by 0x4E3E14A: transfer_threadproc (in /usr/local/lib/libhackrf.so.0.5.0)
==32509==    by 0x54556B9: start_thread (pthread_create.c:333)
==32509==    by 0x598841C: clone (clone.S:109)
==32509==  If you believe this happened as a result of a stack
==32509==  overflow in your program's main thread (unlikely but
==32509==  possible), you can try to increase the size of the
==32509==  main thread stack using the --main-stacksize= flag.
==32509==  The main thread stack size used in this run was 8388608.
==32509== 
==32509== HEAP SUMMARY:
==32509==     in use at exit: 0 bytes in 0 blocks
==32509==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==32509== 
==32509== All heap blocks were freed -- no leaks are possible
==32509== 
==32509== For counts of detected and suppressed errors, rerun with: -v
==32509== ERROR SUMMARY: 2 errors from 1 contexts (suppressed: 0 from 0)
Killed

I have gotten this to "work" as it runs to completion, but I aways get the invalid/uninitialized read error on line 88 when I go to use the context. When it does work, any attempt to access ctx.context causes the error.

How should I go about doing this? I think I need to put my CallbackContext on the heap so that it survives the call to hackrf_start_rx. However, I'm not 100% sure how to clean it up after. Thoughts? Thanks!

Riateche · January 17, 2019, 5:11pm

Not sure what's causing the problem, but why do you need context: Option<&'a mut T>? A lambda by itself can capture any context it needs, so there is no need to pass variables explicitly.

kornel · January 17, 2019, 5:27pm

Instead of Box::leak it's better to use:
1. Box::into_raw to create a pointer,
2. *mut/*const and ptr.as_ref() to use the pointer without destroying it,
3. and finally, Box::from_raw to free the data.
Your code either doesn't need to use Box, or has a use-after-free bug. Box<struct with &mut> doesn't make sense, because even though the Box is long-lived, the &mut reference is only temporary. You can't make &mut non-temporary.

If the FFI code could possibly use the callback after start_rx returns, then you can't use any & or &mut anywhere in the data that FFI can see. You must copy or own everything, e.g. Box<dyn FnMut….

If the FFI code certainly won't use the callback after hackrf_start_rx, then you don't need Box at all. You can pass reference to a local variable.

wspeirs · January 18, 2019, 12:22am

@Riateche I wasn't sure about the function capturing context. I wasn't sure how the compiler would know how to deal with captured context being cast to a c_void pointer, and back again. Thought it better to explicitly pass some context... but I'll give this approach a shot, as it's cleaner.

@kornel taking your advice as well, from point #2, I'm left with the following code that doesn't compile now:

    unsafe extern "C" fn rx_callback(transfer: *mut hackrf_transfer) -> i32 {
        // construct a slice given the pointer and valid length
        let buffer :&[u8] = slice::from_raw_parts((*transfer).buffer, (*transfer).valid_length as usize);

        // construct the context, "casting" back to a CallbackContext
        let ctx = (*transfer).rx_ctx;

        let callback= &mut *(ctx as *mut FnMut(&[u8]) -> Error);

        // call the function, and convert the Error into an i32
        Into::into(callback(buffer))
    }

    pub fn start_rx(&mut self, callback: &mut FnMut(&[u8]) -> Error) -> Result<(), Error> {
        unsafe {
            let ctx = (callback as *mut FnMut(&[u8]) -> Error) as *mut c_void;

            // call the underlying function
            let ret = hackrf_start_rx(self.device_ptr, Some(Device::rx_callback), ctx);

            if ret != hackrf_error_HACKRF_SUCCESS {
                return Err(Error::from(ret));
            }
        }

        // set the state so we can cleanup properly
        self.state = State::RECEIVING;

        Ok( () )
    }

I get the following compiler error:

error[E0277]: expected a `std::ops::FnMut<(&[u8],)>` closure, found `std::ffi::c_void`
  --> src/device.rs:80:30
   |
80 |         let callback= &mut *(ctx as *mut FnMut(&[u8]) -> Error);
   |                              ^^^ expected an `FnMut<(&[u8],)>` closure, found `std::ffi::c_void`
   |
   = help: the trait `for<'r> std::ops::FnMut<(&'r [u8],)>` is not implemented for `std::ffi::c_void`
   = note: required for the cast to the object type `dyn for<'r> std::ops::FnMut(&'r [u8]) -> error::Error`

This is confusing because I would have thought that I could have cast a c_void (which ctx is) to any *mut I wanted... but for some reason this isn't working. Thoughts? Thanks!

kornel · January 18, 2019, 12:25am

References to closures aren't pointers. They are structs that contain function pointer and a context pointer.

However you can take address of the reference (reference of reference of closure) and cast that to a pointer.

wspeirs · January 18, 2019, 2:28am

@kornel, the problem I'm having is not going from closure to *mut c_void, the problem I'm having is going from *mut c_void to closure:

80 |         let callback= &mut *(ctx as *mut FnMut(&[u8]) -> Error);
   |                              ^^^ expected an `FnMut<(&[u8],)>` closure, found `std::ffi::c_void`
   |
   = help: the trait `for<'r> std::ops::FnMut<(&'r [u8],)>` is not implemented for `std::ffi::c_void`
   = note: required for the cast to the object type `dyn for<'r> std::ops::FnMut(&'r [u8]) -> error::Error`

In the above, ctx is a *mut c_void: let ctx :*mut c_void = (*transfer).rx_ctx;.

wspeirs · January 18, 2019, 3:30am

Taking another stab at this, going off this SO post, I'm left with:

    unsafe extern "C" fn rx_callback(transfer: *mut hackrf_transfer) -> i32 {
        let buffer :&[u8] = slice::from_raw_parts((*transfer).buffer, (*transfer).valid_length as usize);

        let ctx :*mut c_void = (*transfer).rx_ctx;
        let callback :&mut &mut FnMut(&[u8]) -> Error = transmute(ctx);

        Into::into(callback(buffer.as_slice())) // <- device.rs:91, see Valgrind error
    }

    pub fn start_rx<F>(&mut self, mut callback: F) -> Result<(), Error>
    where F: FnMut(&[u8]) -> Error {
        unsafe {
            let mut trait_obj: &mut FnMut(&[u8]) -> Error = &mut callback;
            let ctx :*mut c_void = transmute(&mut trait_obj);

            let ret = hackrf_start_rx(self.device_ptr, Some(Device::rx_callback), ctx);
        }

        Ok( () )
    }

However, I'm still getting a crash and Valgrind is reporting uninit reads:

==9809== Thread 4:
==9809== Use of uninitialised value of size 8
==9809==    at 0x175B9C: core::ops::function::impls::_$LT$impl$u20$core..ops..function..FnMut$LT$A$GT$$u20$for$u20$$RF$mut$u20$F$GT$::call_mut::h56ab5d92929ca72f (function.rs:276)
==9809==    by 0x13634E: rs_libhackrf::device::Device::rx_callback::ha10f0099c22d419d (device.rs:91)
==9809==    by 0x4E3E21C: hackrf_libusb_transfer_callback (in /usr/local/lib/libhackrf.so.0.5.0)
==9809==    by 0x5F5D159: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==9809==    by 0x5F62F07: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==9809==    by 0x5F5CC48: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==9809==    by 0x5F5DB52: libusb_handle_events_timeout_completed (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==9809==    by 0x4E3E14A: transfer_threadproc (in /usr/local/lib/libhackrf.so.0.5.0)
==9809==    by 0x54556B9: start_thread (pthread_create.c:333)
==9809==    by 0x598841C: clone (clone.S:109)
==9809==  Uninitialised value was created by a stack allocation
==9809==    at 0x545EC12: ??? (syscall-template.S:84)

It complains the value was created on the stack, which makes sense because trait_obj lives on the stack. I tried to get around this by simply doing: let ctx :*mut c_void = transmute(&mut &mut callback); but I get into the same situation, an uninit read from a value created via stack allocation.

I'm starting to wonder if this is possible at all: create a *mut c_void that points at a closure, and is passed through a C void * where it can be re-cast back to the original closure. Thoughts? Thanks!

vitalyd · January 18, 2019, 3:55am

The callback in start_rx is dropped at the end of that function, and hence doesn’t live long enough for your rx thread to call it back properly.

If you don’t have an existing stack or heap location to hold a stable address for your callback, you need to allocate it on the heap (eg get a Box<Box<dyn FnMut(&[u8]) -> Error>>), then Box::into_raw that so it doesn’t get dropped, store the resulting *mut *mut ... in the context, restore back to that Box (via Box::from_raw) in the rx callback, invoke the closure, and let the box drop to deallocate.

I’m on mobile so a bit terse above - sorry.

wspeirs · January 18, 2019, 12:25pm

@vitalyd, thanks for the help! Makes sense that the callback parameter to start_rx is dropped at the end of the call to start_rx. I changed it so that the boxing happens outside this function, but I'm still missing something:

     unsafe extern "C" fn rx_callback(transfer: *mut hackrf_transfer) -> i32 {
        let buffer :&[u8] = slice::from_raw_parts((*transfer).buffer, (*transfer).valid_length as usize);

        let ctx :*mut c_void = (*transfer).rx_ctx;
        let callback :*mut Box<FnMut(&[u8]) -> Error> = transmute(ctx);
        let mut callback = Box::from_raw(callback);

        Into::into((*(*callback))(buffer.as_slice()))  // <- device.rs:92, see Valgrind output
    }

    pub fn start_rx<F>(&mut self, callback: Box<Box<F>>) -> Result<(), Error>
    where F: FnMut(&[u8]) -> Error {
        unsafe {
            let ctx :*mut c_void = transmute(Box::into_raw(callback));

            let ret = hackrf_start_rx(self.device_ptr, Some(Device::rx_callback), ctx);

            if ret != hackrf_error_HACKRF_SUCCESS {
                return Err(Error::from(ret));
            }
        }
        Ok( () )
    }

I simply call into_raw on callback which should create a temporary, but that temp only holds the location of the outer Box pointer. (Not 100% sure why I need the double-Box, but going with it.) This temporary is then "cast" into a *mut c_void and passed as a parameter... which all makes sense.

On the unpacking side, I'm converting from *mut c_void into *mut Box<F>, which I then construct into a Box via Box::from_raw, which should leave me with my original Box<Box<F>>. I double-deref so I can get the function F and call it. The result, a crash:

==32213== Thread 4:
==32213== Invalid read of size 8
==32213==    at 0x136417: rs_libhackrf::device::Device::rx_callback::ha10f0099c22d419d (device.rs:92)
==32213==    by 0x4E3E21C: hackrf_libusb_transfer_callback (in /usr/local/lib/libhackrf.so.0.5.0)
==32213==    by 0x5F5D159: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32213==    by 0x5F62F07: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32213==    by 0x5F5CC48: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32213==    by 0x5F5DB52: libusb_handle_events_timeout_completed (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32213==    by 0x4E3E14A: transfer_threadproc (in /usr/local/lib/libhackrf.so.0.5.0)
==32213==    by 0x54556B9: start_thread (pthread_create.c:333)
==32213==    by 0x598841C: clone (clone.S:109)
==32213==  Address 0x4d0000352d4c is not stack'd, malloc'd or (recently) free'd
==32213== 
==32213== 
==32213== Process terminating with default action of signal 11 (SIGSEGV)
==32213==  Access not within mapped region at address 0x18
==32213==    at 0x136417: rs_libhackrf::device::Device::rx_callback::ha10f0099c22d419d (device.rs:92)
==32213==    by 0x4E3E21C: hackrf_libusb_transfer_callback (in /usr/local/lib/libhackrf.so.0.5.0)
==32213==    by 0x5F5D159: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32213==    by 0x5F62F07: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32213==    by 0x5F5CC48: ??? (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32213==    by 0x5F5DB52: libusb_handle_events_timeout_completed (in /lib/x86_64-linux-gnu/libusb-1.0.so.0.1.0)
==32213==    by 0x4E3E14A: transfer_threadproc (in /usr/local/lib/libhackrf.so.0.5.0)
==32213==    by 0x54556B9: start_thread (pthread_create.c:333)
==32213==    by 0x598841C: clone (clone.S:109)

So clearly I'm de-referencing something improperly. Thoughts on all of this? Is my logic of passing Box<Box<F>> into my function sound? Ergonomically not great, but at this point I'm simply looking for something that'll run without crashing or an uninitialized read. Thoughts? Thanks!

vitalyd · January 18, 2019, 1:57pm

The problem here is you have Box<Box<F>>, with a concrete F: FnMut, which isn't a trait object, but you're casting it back to a trait object in rx_callback, and calling it as-if it's a trait object.

You need to have a Box<Box<dyn FnMut(&[u8])>> on both sides. Here is a contrived example to hopefully help you.

wspeirs · January 19, 2019, 3:49am

@vitalyd, thanks again for the help! I took your example, and modified it to more closely fit what I'm trying to do, but still running into an issue, that I honestly don't think there is a great solution to. When I'm in my call-back rx_callback I'm converting the *mut c_void into a Box using Box::from_raw; however, when the function returns and this is dropped, the memory is freed, but I still need this pointer to be there the next time rx_callback is called from the thread! A simplified version of my code:

    unsafe extern "C" fn rx_callback(transfer: *mut hackrf_transfer) -> i32 {
        // get the callback context out of the transfer struct
        let callback :*mut c_void = (*transfer).rx_ctx;
        let mut callback :Box<Box<dyn FnMut(&[u8]) -> Error>> = Box::from_raw(callback as _);
        let callback = callback.as_mut().as_mut();

        // call the function
        Into::into(callback(buffer))  // the problem is that buffer is dropped here, freeing the memory!!!
    }

    pub fn start_rx<F>(&mut self, callback: F) -> Result<(), Error>
    where F: FnMut(&[u8]) -> Error
    {
        unsafe {
            let ctx :Box<dyn FnMut(&[u8]) -> Error> = Box::new(callback);
            let ctx = Box::new(ctx);
            let ctx :*mut c_void = Box::into_raw(ctx) as _;

            // this function starts a pthread that calls rx_callback multiple times
            let ret = hackrf_start_rx(self.device_ptr, Some(Device::rx_callback), ctx);

            if ret != hackrf_error_HACKRF_SUCCESS {
                return Err(Error::from(ret));
            }
        }

        Ok( () )
    }

The above causes a seg fault in jemalloc... I believe because of multiple frees.

One thought I had was instead of using Box::from_raw I need to cast the pointer somehow, then when I know there are no more calls to rx_callback I can capture the pointer, and call Box::from_raw, and then drop it. Thoughts? Thanks!

wspeirs · January 19, 2019, 4:17am

Yea, so if I use Box::leak I can get it to work without any errors, but of course I'm leaking memory...

        let callback :*mut c_void = (*transfer).rx_ctx;
        let mut callback :Box<Box<dyn FnMut(&[u8]) -> Error>> = Box::from_raw(callback as _);
        let callback = Box::leak(callback);
        let callback = callback.as_mut();

        // call the function, and convert the Error into an i32
        Into::into(callback(buffer))

... but from Valgrind:

==12320== HEAP SUMMARY:
==12320==     in use at exit: 56 bytes in 3 blocks
==12320==   total heap usage: 4,121 allocs, 4,118 frees, 3,116,559 bytes allocated
==12320== 
==12320== LEAK SUMMARY:
==12320==    definitely lost: 16 bytes in 1 blocks
==12320==    indirectly lost: 0 bytes in 0 blocks
==12320==      possibly lost: 0 bytes in 0 blocks
==12320==    still reachable: 40 bytes in 2 blocks

Haven't tried capturing the pointer, and freeing it later... that's the next step I guess. I'm just happy to see zero errors in Valgrind

vitalyd · January 19, 2019, 4:17am

Ah, yes - I didn’t realize the callback is used more than once, thought it was a one-shot type of deal.

So yeah, don’t create the Box from the *mut c_void there, wait till you’re fully done as you said. Cast the raw ptr to *mut *mut dyn FnMut(...), and then coerce that to a borrowed trait obj, ie:

let callback = (*transfer).rx_ctx as *mut *mut dyn FnMut(&[u8]) -> Error;
let callback = unsafe { &mut **callback };
Into::into(callback(buffer))

wspeirs · January 19, 2019, 4:19am

@vitalyd, just tried the above, same net effect... no errors, but leaked memory. Capturing the pointer and freeing it later is a task for tomorrow. Thanks again for all the help!!!

vitalyd · January 19, 2019, 4:20am

Yeah, you’ll need to free manually when it’s the right time. Doing the coercion there just saves you the silly Box leak dance.

No problem - good luck!

Topic		Replies	Views
How to convert a Rust closure to a C-style callback? help	29	4001	July 31, 2020
Struggles creating a C wrapper for rust lib help	17	1130	October 8, 2022
Scope for structs and raw pointers for FFI/C library help	3	716	February 9, 2021
Box from and to raw (SIGSEV - address not mapped to object) help	12	312	November 17, 2025
FFI Callbacks accessing outer variables help	14	1665	September 17, 2020

Casting c_void* for C-Callback

Related topics