Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement rust MainThread #5671

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ pub mod interaction;
pub mod linearview;
pub mod llil;
pub mod logger;
pub mod mainthread;
pub mod metadata;
pub mod mlil;
pub mod platform;
Expand Down
204 changes: 204 additions & 0 deletions rust/src/mainthread.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
//! This module provides two ways to execute "jobs":
//!
//! 1. On the Binary Ninja main thread (the UI event thread when running in the GUI application):
//! * [execute_on_main_thread]
//! * [execute_on_main_thread_and_wait]
//! 2. On a worker thread
//!
//! Any manipulation of the GUI should be performed on the main thread, but any
//! non-GUI work is generally better to be performed using a worker. This is
//! especially true for any longer-running work, as the user interface will
//! be unable to update itself while a job is executing on the main thread.
//!
//! There are three worker queues, in order of decreasing priority:
//!
//! 1. The Interactive Queue ([worker_interactive_enqueue])
//! 2. The Priority Queue ([worker_priority_enqueue])
//! 3. The Worker Queue ([worker_enqueue])
//!
//! All of these queues are serviced by the same pool of worker threads. The
//! difference between the queues is basically one of priority: one queue must
//! be empty of jobs before a worker thread will execute a job from a lower
//! priority queue.
//!
//! The default maximum number of concurrent worker threads is controlled by the
//! `analysis.limits.workerThreadCount` setting but can be adjusted at runtime via
//! [set_worker_thread_count].
//!
//! The worker threads are native threads, managed by the Binary Ninja core. If
//! more control over the thread is required, consider using the
//! [crate::backgroundtask::BackgroundTask] class.

use core::{ffi, mem, ptr};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there's no realistic chance of ever being #![no_std], these should be imported from std instead. Also, types like NonNull, CStr, c_char, etc. should probably be imported here as well to avoid extra :: when using those types (they're used a lot in this file).


use binaryninjacore_sys::*;

use crate::string::BnStrCompatible;

#[repr(transparent)]
pub struct MainThreadAction {
handle: ptr::NonNull<BNMainThreadAction>,
Copy link
Contributor

@mkrasnitski mkrasnitski Jul 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the documentation for NonNull mentions that it's covariant as opposed to *mut T which is invariant. This worries me a little bit especially because much of the Binja Rust bindings offer some amount of interior mutability by having methods take &self while calling C functions that mutate the underlying data. I'm unsure of the soundness here, but I think it's probably fine because the only data held on the Rust side is the raw handle. Although, since we're talking about a concrete NonNull and not a generic NonNull<T>, then variance probably doesn't even matter.

}

impl Clone for MainThreadAction {
fn clone(&self) -> Self {
let result = unsafe { BNNewMainThreadActionReference(self.as_raw()) };
unsafe { Self::from_raw(ptr::NonNull::new(result).unwrap()) }
}
}

impl Drop for MainThreadAction {
fn drop(&mut self) {
unsafe { BNFreeMainThreadAction(self.as_raw()) }
}
}

impl MainThreadAction {
pub(crate) unsafe fn from_raw(handle: ptr::NonNull<BNMainThreadAction>) -> Self {
Self { handle }
}

pub(crate) unsafe fn ref_from_raw(handle: &*mut BNMainThreadAction) -> &Self {
assert!(!handle.is_null());
mem::transmute(handle)
}
Comment on lines +61 to +64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this function actually used anywhere? And since *mut T: Copy, you could reasonably just call from_raw instead and get an owned Self. Plus, elsewhere in the codebase, the return type of ref_from_raw is usually Ref<Self>. I also don't like the use of transmute here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is used by the cb_add_action. The copy is intentionally avoided here, to avoid problems like #5496

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, a better alternative to raw transmute is something like:

unsafe { &*(handle as *const _ as *const Self) }

It's more clear what's going on here and it doesn't tap into transmute's full power (which is a good thing)


#[allow(clippy::mut_from_ref)]
pub(crate) unsafe fn as_raw(&self) -> &mut BNMainThreadAction {
&mut *self.handle.as_ptr()
}
Comment on lines +66 to +69
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#[allow(clippy::mut_from_ref)]
pub(crate) unsafe fn as_raw(&self) -> &mut BNMainThreadAction {
&mut *self.handle.as_ptr()
}
pub(crate) fn as_raw(&self) -> *mut BNMainThreadAction {
self.handle.as_ptr()
}

This function isn't pub and passing the raw *mut BNMainThreadAction across FFI already requires unsafe, so this function should just be a wrapper around NonNull::as_ptr. As far as I can tell, every use of this function coerces the return type from &mut to *mut.

Copy link
Contributor Author

@rbran rbran Jul 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to above, the *mut T: Copy is avoided and lifetime is enforced to avoid situations like #5496.

Although the reference is usually immediately converted into a pointer, voiding the effort of enforcing the lifetime. In any case, this is a internal function and it could be fixed without changing the API.


pub fn execute(&self) {
unsafe { BNExecuteMainThreadAction(self.as_raw()) }
}

pub fn is_done(&self) -> bool {
unsafe { BNIsMainThreadActionDone(self.as_raw()) }
}

pub fn wait(&self) {
unsafe { BNWaitForMainThreadAction(self.as_raw()) }
}
}

/// Takes a single parameter which is a function that will be executed
/// on the main Binary Ninja thread.
///
/// <div class="warning">
///
/// May be required for some GUI operations, but should be used sparingly as it can block the UI.
///
/// </div>
pub fn execute_on_main_thread<F: FnMut() + 'static>(action: F) -> Option<MainThreadAction> {
let action = Box::leak(Box::new(action));
let result = unsafe {
BNExecuteOnMainThread(
action as *mut F as *mut ffi::c_void,
Some(cb_execute_with_drop::<F>),
)
};
ptr::NonNull::new(result).map(|h| unsafe { MainThreadAction::from_raw(h) })
}

/// Takes a single parameter which is a function that will be
/// executed on the main Binary Ninja thread and will block execution of further python until the function returns.
///
/// <div class="warning">
///
/// May be required for some GUI operations, but should be used sparingly as it can block the UI.
///
/// </div>
pub fn execute_on_main_thread_and_wait<F: FnMut() + 'static>(mut action: F) {
unsafe {
BNExecuteOnMainThreadAndWait(
&mut action as *mut F as *mut ffi::c_void,
Some(cb_execute_without_drop::<F>),
)
}
}

pub fn worker_enqueue<F: FnMut() + 'static, S: BnStrCompatible>(action: F, name: S) {
let action = Box::leak(Box::new(action));
let name = name.into_bytes_with_nul();
unsafe {
BNWorkerEnqueueNamed(
action as *mut F as *mut ffi::c_void,
Some(cb_execute_with_drop::<F>),
name.as_ref().as_ptr() as *const ffi::c_char,
)
}
}

pub fn worker_priority_enqueue<F: FnMut() + 'static, S: BnStrCompatible>(action: F, name: S) {
let action = Box::leak(Box::new(action));
let name = name.into_bytes_with_nul();
unsafe {
BNWorkerPriorityEnqueueNamed(
action as *mut F as *mut ffi::c_void,
Some(cb_execute_with_drop::<F>),
name.as_ref().as_ptr() as *const ffi::c_char,
)
}
}

pub fn worker_interactive_enqueue<F: FnMut() + 'static, S: BnStrCompatible>(action: F, name: S) {
let action = Box::leak(Box::new(action));
let name = name.into_bytes_with_nul();
unsafe {
BNWorkerInteractiveEnqueueNamed(
action as *mut F as *mut ffi::c_void,
Some(cb_execute_with_drop::<F>),
name.as_ref().as_ptr() as *const ffi::c_char,
)
}
}

/// Returns the number of worker threads that are currently running.
/// By default, this is the number of cores on the system minus one, however this can be changed with
/// [set_worker_thread_count].
pub fn get_worker_thread_count() -> usize {
unsafe { BNGetWorkerThreadCount() }
}

/// Sets the number of worker threads that are currently running.
/// By default, this is the number of cores on the system minus one.
pub fn set_worker_thread_count(count: usize) {
unsafe { BNSetWorkerThreadCount(count) }
}

pub fn is_main_thread() -> bool {
unsafe { BNIsMainThread() }
}

pub trait MainThreadProvider {
fn add_action(&self, action: &MainThreadAction);
}

pub fn register_main_thread<T: MainThreadProvider>(action: T) {
// SAFETY new main thread provider is never freed
let context = Box::leak(Box::new(action));
let mut callback = BNMainThreadCallbacks {
context: context as *mut T as *mut ffi::c_void,
addAction: Some(cb_add_action::<T>),
};
unsafe { BNRegisterMainThread(&mut callback) }
}

unsafe extern "C" fn cb_add_action<T: MainThreadProvider>(
ctxt: *mut ffi::c_void,
action: *mut BNMainThreadAction,
) {
let ctxt = &mut *(ctxt as *mut T);
ctxt.add_action(MainThreadAction::ref_from_raw(&action))
}

unsafe extern "C" fn cb_execute_without_drop<F: FnMut() + 'static>(ctxt: *mut ffi::c_void) {
let ctxt = &mut *(ctxt as *mut F);
ctxt()
}

unsafe extern "C" fn cb_execute_with_drop<F: FnMut() + 'static>(ctxt: *mut ffi::c_void) {
let mut ctxt = Box::from_raw(ctxt as *mut F);
ctxt();
drop(ctxt);
}
Loading