Simplify callback handling logic
We're leaking them anyway, no need to keep a global synchronized slab
MODIFY
Cargo.toml
Cargo.toml
@@ -23,7 +23,6 @@ members = ["binding-generator"]
libc = "0.2"
num = "0.2"
once_cell = "1.0"
-slab = "0.4"
[features]
default = ["opencv-4"]

MODIFY
README.md
README.md
@@ -296,14 +296,11 @@ for example you pass "123\0456" to the setter, the property will be set to "123"
### Callbacks
-Some API functions accept callbacks, e.g. set_mouse_callback. While currently
-it's possible to successfully use those functions there are some limitations to
-keep in mind. Current implementation of callback handling keeps hold of the
-passed callback argument forever. That means that the closure used as a callback
-will never be freed during the lifetime of a program and moreover Drop will
-not be called for it (they are stored in global static [`Slab`](https://crates.io/crates/slab)).
-There is a plan to implement possibility to be able to free at least some of the
-closures.
+Some API functions accept callbacks, e.g. `set_mouse_callback`. While currently it's possible to successfully
+use those functions there are some limitations to keep in mind. Current implementation of callback handling
+leaks the passed callback argument. That means that the closure used as a callback will never be freed during
+the lifetime of a program and moreover Drop will not be called for it. There is a plan to implement possibility
+to be able to free at least some of the closures.
### Unsafety

MODIFY
binding-generator/src/function.rs
binding-generator/src/function.rs
@@ -56,7 +56,7 @@ impl<'tu, 'g> Function<'tu, 'g> {
.map(|a| a.type_ref().rust_extern().into_owned())
.join(", ");
let ret = self.return_type();
- format!(r#"Option<extern "C" fn({args}) -> {ret}>"#, args=args, ret=ret.rust_extern()).into()
+ format!(r#"Option<unsafe extern "C" fn({args}) -> {ret}>"#, args=args, ret=ret.rust_extern()).into()
}
}

MODIFY
src/templ.rs
src/templ.rs
@@ -31,13 +31,11 @@ macro_rules! string_arg_output_receive {
macro_rules! callback_arg {
($tr_name: ident($($tr_arg_name: ident: $tr_arg_type: ty),*) -> $tr_ret: ty => $tr_userdata_name: ident in $callbacks_name: ident => $callback_name: ident($($fw_arg_name: ident: $fw_arg_type: ty),*) -> $fw_ret: ty) => {
- static $callbacks_name: ::once_cell::sync::Lazy<::std::sync::Mutex<::slab::Slab<Box<dyn FnMut($($fw_arg_type),*) -> $fw_ret + Send + Sync>>>> = ::once_cell::sync::Lazy::new(|| ::std::sync::Mutex::new(::slab::Slab::with_capacity(1)));
- extern "C" fn trampoline($($tr_arg_name: $tr_arg_type),*) -> $tr_ret {
- if let Some(callback) = $callbacks_name.lock().unwrap().get_mut($tr_userdata_name as _) {
- callback($($fw_arg_name),*)
- } else {
- panic!("Callback not found!") // really bad, fixme somehow
- }
+ unsafe extern "C" fn trampoline($($tr_arg_name: $tr_arg_type),*) -> $tr_ret {
+ let mut callback: Box<Box<dyn FnMut($($fw_arg_type),*) -> $fw_ret + Send + Sync>> = Box::from_raw($tr_userdata_name as _);
+ let out = callback($($fw_arg_name),*);
+ Box::into_raw(callback);
+ out
}
let $tr_name = if $callback_name.is_some() {
@@ -51,7 +49,7 @@ macro_rules! callback_arg {
macro_rules! userdata_arg {
($userdata_name: ident in $callbacks_name: ident => $callback_name: ident) => {
let $userdata_name = if let Some(callback) = $callback_name {
- $callbacks_name.lock().unwrap().insert(callback) as *mut ::std::ffi::c_void
+ Box::into_raw(Box::new(callback)) as *mut ::std::ffi::c_void
} else {
0 as _ // fixme, remove previous callback
};

Simplify callback handling logic

We're leaking them anyway, no need to keep a global synchronized slab