-
Notifications
You must be signed in to change notification settings - Fork 274
Description
Current state of data notifications in Rust API:
binaryninja-api/rust/tests/data_notification.rs
Lines 16 to 33 in 6e50ced
| let custom = DataNotificationClosure::default() | |
| .function_updated(|_bv: &BinaryView, _func: &Function| { | |
| func_updated_count += 1; | |
| }) | |
| .register(&bv); | |
| let crash = bv.create_tag_type("Test", "🚧"); | |
| let funcs = bv.functions(); | |
| for func in &funcs { | |
| func.add_tag( | |
| &crash, | |
| "Dummy tag", | |
| Some(10.try_into().unwrap()), | |
| true, | |
| None, | |
| ); | |
| } | |
| custom.unregister(); |
The custom is of type DataNotificationHandle which is defined as:
binaryninja-api/rust/src/data_notification.rs
Lines 414 to 421 in ca0e32e
| pub struct DataNotificationHandle<'a, T: CustomDataNotification> | |
| where | |
| T: 'a, | |
| { | |
| bv: Ref<BinaryView>, | |
| handle: Box<BNBinaryDataNotification>, | |
| _life: std::marker::PhantomData<&'a T>, | |
| } |
This means that we are holding a strong reference to the view (for the purposes of dropping the data notification when custom is dropped) which inhibits the ability to leak the custom handle to persist the data notifications, we need to address this somehow to make using data notifications in Rust API more user friendly and less of a footgun.
One idea would be to provide an alternate register API that does not return a handle containing a strong reference to the view, or instead hold a weak reference to the view, but that is very difficult to do.
See this example for a motivating reason.