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

Add MainThreadToken to ensure file-dialogs only run on the main thread #8467

Merged
merged 11 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
13 changes: 13 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5592,6 +5592,15 @@ dependencies = [
"walkdir",
]

[[package]]
name = "re_capabilities"
version = "0.21.0-alpha.1+dev"
dependencies = [
"document-features",
"egui",
"static_assertions",
]

[[package]]
name = "re_case"
version = "0.21.0-alpha.1+dev"
Expand Down Expand Up @@ -5788,6 +5797,7 @@ dependencies = [
"image",
"itertools 0.13.0",
"nohash-hasher",
"re_capabilities",
"re_chunk_store",
"re_entity_db",
"re_format",
Expand Down Expand Up @@ -6785,6 +6795,7 @@ dependencies = [
"re_blueprint_tree",
"re_build_info",
"re_build_tools",
"re_capabilities",
"re_chunk",
"re_chunk_store",
"re_chunk_store_ui",
Expand Down Expand Up @@ -6866,6 +6877,7 @@ dependencies = [
"nohash-hasher",
"once_cell",
"parking_lot",
"re_capabilities",
"re_chunk",
"re_chunk_store",
"re_data_source",
Expand Down Expand Up @@ -7166,6 +7178,7 @@ dependencies = [
"re_analytics",
"re_build_info",
"re_build_tools",
"re_capabilities",
"re_chunk",
"re_chunk_store",
"re_crash_handler",
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ rerun-cli = { path = "crates/top/rerun-cli", version = "=0.21.0-alpha.1", defaul

# crates/utils:
re_analytics = { path = "crates/utils/re_analytics", version = "=0.21.0-alpha.1", default-features = false }
re_capabilities = { path = "crates/utils/re_capabilities", version = "=0.21.0-alpha.1", default-features = false }
re_case = { path = "crates/utils/re_case", version = "=0.21.0-alpha.1", default-features = false }
re_crash_handler = { path = "crates/utils/re_crash_handler", version = "=0.21.0-alpha.1", default-features = false }
re_error = { path = "crates/utils/re_error", version = "=0.21.0-alpha.1", default-features = false }
Expand Down
8 changes: 7 additions & 1 deletion crates/top/rerun-cli/src/bin/rerun.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,17 @@ fn main() -> std::process::ExitCode {
}

fn main_impl() -> std::process::ExitCode {
let main_thread_token = rerun::MainThreadToken::i_promise_i_am_on_the_main_thread();
re_log::setup_logging();

let build_info = re_build_info::build_info!();

let result = rerun::run(build_info, rerun::CallSource::Cli, std::env::args());
let result = rerun::run(
main_thread_token,
build_info,
rerun::CallSource::Cli,
std::env::args(),
);

match result {
Ok(exit_code) => std::process::ExitCode::from(exit_code),
Expand Down
3 changes: 2 additions & 1 deletion crates/top/rerun/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -124,18 +124,19 @@ web_viewer = ["server", "dep:re_web_viewer_server", "re_sdk?/web_viewer"]

[dependencies]
re_build_info.workspace = true
re_capabilities.workspace = true
re_chunk.workspace = true
re_crash_handler.workspace = true
re_entity_db.workspace = true
re_error.workspace = true
re_format.workspace = true
re_log_encoding.workspace = true
re_log_types.workspace = true
re_video.workspace = true
re_log.workspace = true
re_memory.workspace = true
re_smart_channel.workspace = true
re_tracing.workspace = true
re_video.workspace = true

anyhow.workspace = true
document-features.workspace = true
Expand Down
6 changes: 5 additions & 1 deletion crates/top/rerun/src/commands/entrypoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,7 @@ enum Command {
// It would be nice to use [`std::process::ExitCode`] here but
// then there's no good way to get back at the exit code from python
pub fn run<I, T>(
main_thread_token: crate::MainThreadToken,
build_info: re_build_info::BuildInfo,
call_source: CallSource,
args: I,
Expand Down Expand Up @@ -595,7 +596,7 @@ where
}
}
} else {
run_impl(build_info, call_source, args)
run_impl(main_thread_token, build_info, call_source, args)
};

match res {
Expand All @@ -620,6 +621,7 @@ where
}

fn run_impl(
main_thread_token: crate::MainThreadToken,
_build_info: re_build_info::BuildInfo,
call_source: CallSource,
args: Args,
Expand Down Expand Up @@ -836,8 +838,10 @@ fn run_impl(
} else {
#[cfg(feature = "native_viewer")]
return re_viewer::run_native_app(
main_thread_token,
Box::new(move |cc| {
let mut app = re_viewer::App::new(
main_thread_token,
_build_info,
&call_source.app_env(),
startup_options,
Expand Down
2 changes: 2 additions & 0 deletions crates/top/rerun/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ pub use re_entity_db::external::re_chunk_store::{
};
pub use re_log_types::StoreKind;

pub use re_capabilities::MainThreadToken;

/// To register a new external data loader, simply add an executable in your $PATH whose name
/// starts with this prefix.
// NOTE: this constant is duplicated in `re_data_source` to avoid an extra dependency here.
Expand Down
6 changes: 5 additions & 1 deletion crates/top/rerun/src/native_viewer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ use re_log_types::LogMsg;
///
/// ⚠️ This function must be called from the main thread since some platforms require that
/// their UI runs on the main thread! ⚠️
pub fn show(msgs: Vec<LogMsg>) -> re_viewer::external::eframe::Result {
pub fn show(
main_thread_token: crate::MainThreadToken,
msgs: Vec<LogMsg>,
) -> re_viewer::external::eframe::Result {
if msgs.is_empty() {
re_log::debug!("Empty array of msgs - call to show() ignored");
return Ok(());
Expand All @@ -18,6 +21,7 @@ pub fn show(msgs: Vec<LogMsg>) -> re_viewer::external::eframe::Result {

let startup_options = re_viewer::StartupOptions::default();
re_viewer::run_native_viewer_with_messages(
main_thread_token,
re_build_info::build_info!(),
re_viewer::AppEnvironment::from_store_source(&store_source),
startup_options,
Expand Down
35 changes: 35 additions & 0 deletions crates/utils/re_capabilities/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
[package]
name = "re_capabilities"
authors.workspace = true
description = "Capability tokens for the Rerun code base."
edition.workspace = true
homepage.workspace = true
include.workspace = true
license.workspace = true
publish = true
readme = "README.md"
repository.workspace = true
rust-version.workspace = true
version.workspace = true

[lints]
workspace = true

[package.metadata.docs.rs]
all-features = true


[features]
default = []

## Enable constructing the [`MainThreadToken`] from an [`egui::Ui`].
egui = ["dep:egui"]


[dependencies]
# Internal dependencies:

# External dependencies:
document-features.workspace = true
egui = { workspace = true, default-features = false, optional = true }
static_assertions.workspace = true
11 changes: 11 additions & 0 deletions crates/utils/re_capabilities/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# re_capabilities

Part of the [`rerun`](https://github.com/rerun-io/rerun) family of crates.

[![Latest version](https://img.shields.io/crates/v/re_capabilities.svg)](https://crates.io/crates/utils/re_capabilities)
[![Documentation](https://docs.rs/re_capabilities/badge.svg)](https://docs.rs/re_capabilities)
![MIT](https://img.shields.io/badge/license-MIT-blue.svg)
![Apache](https://img.shields.io/badge/license-Apache-blue.svg)

Specifies capability tokens, required by different parts of the code base.
These are tokens passed down the call tree, to explicitly allow different capabilities in different parts of the code base.
19 changes: 19 additions & 0 deletions crates/utils/re_capabilities/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//! Specifies capability tokens, required by different parts of the code base.
//! These are tokens passed down the call tree, to explicitly allow different capabilities in different parts of the code base.
//!
//! For instance, the [`MainThreadToken`] is taken by argument in functions that needs to run on the main thread.
//! By requiring this token, you guarantee at compile-time that the function is only called on the main thread.
//!
//! All capability tokens should be created in the top-level of the call tree,
//! (i.e. in `fn main`) and passed down to all functions that require it.
//! That way you can be certain in what an area of code is allowed to do.
//!
//! See [`cap-std`](https://crates.io/crates/cap-std) for another capability-centric crate.
//!
//! ## Feature flags
#![doc = document_features::document_features!()]
//!

mod main_thread_token;

pub use main_thread_token::MainThreadToken;
53 changes: 53 additions & 0 deletions crates/utils/re_capabilities/src/main_thread_token.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
use static_assertions::assert_not_impl_any;

/// A token that proves we are on the main thread.
///
/// Certain operations are only allowed on the main thread.
/// These operations should require this token.
/// For instance, any function using file dialogs (e.g. using [`rfd`](https://docs.rs/rfd/latest/rfd/)) should require this token.
///
/// The token should only be constructed in `fn main`, using [`MainThreadToken::i_promise_i_am_on_the_main_thread`],
/// and then be passed down the call tree to where it is needed.
/// [`MainTheadToken`] is neither `Send` nor `Sync`,
/// thus guaranteeing that it cannot be found in other threads.
///
/// Of course, there is nothing stopping you from calling [`MainThreadToken::i_promise_i_am_on_the_main_thread`] from a background thread,
/// but PLEASE DON'T DO THAT.
/// In other words, don't use this as a guarantee for unsafe code.
///
/// There is also [`MainThreadToken::from_egui_ui`] which uses the implicit guarantee of egui
/// (which _usually_ is run on the main thread) to construct a [`MainThreadToken`].
/// Use this only in a code base where you are sure that egui is running only on the main thread.
#[derive(Clone, Copy)]
pub struct MainThreadToken {
/// Prevent from being sent between threads.
///
/// Workaround until `impl !Send for X {}` is stable.
_dont_send_me: std::marker::PhantomData<*const ()>,
}

impl MainThreadToken {
/// Only call this from `fn main`, or you may get weird runtime errors!
pub fn i_promise_i_am_on_the_main_thread() -> Self {
#[cfg(not(target_arch = "wasm32"))]
debug_assert_eq!(std::thread::current().name(), Some("main"),
emilk marked this conversation as resolved.
Show resolved Hide resolved
"DEBUG ASSERT: Trying to construct a MainThreadToken on a thread that is not the main thread!"
);

Self {
_dont_send_me: std::marker::PhantomData,
}
}

/// We should only create an `egui::Ui` on the main thread,
/// so having it is proof enough that we are on the main thread.
///
/// Use this only in a code base where you are sure that egui is running only on the main thread.
#[cfg(feature = "egui")]
pub fn from_egui_ui(_ui: &egui::Ui) -> Self {
emilk marked this conversation as resolved.
Show resolved Hide resolved
Self::i_promise_i_am_on_the_main_thread()
}
}

assert_not_impl_any!(MainThreadToken: Send, Sync);
assert_not_impl_any!(&MainThreadToken: Send, Sync);
1 change: 1 addition & 0 deletions crates/viewer/re_data_ui/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ workspace = true
all-features = true

[dependencies]
re_capabilities = { workspace = true, features = ["egui"] }
re_chunk_store.workspace = true
re_entity_db.workspace = true
re_format.workspace = true
Expand Down
1 change: 1 addition & 0 deletions crates/viewer/re_data_ui/src/blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ pub fn blob_preview_and_save_ui(
}

ctx.command_sender.save_file_dialog(
re_capabilities::MainThreadToken::from_egui_ui(ui),
&file_name,
"Save blob".to_owned(),
blob.to_vec(),
Expand Down
8 changes: 6 additions & 2 deletions crates/viewer/re_data_ui/src/instance_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,12 @@ fn image_download_button_ui(
.map_or("image", |name| name.unescaped_str())
.to_owned()
);
ctx.command_sender
.save_file_dialog(&file_name, "Save image".to_owned(), png_bytes);
ctx.command_sender.save_file_dialog(
re_capabilities::MainThreadToken::from_egui_ui(ui),
&file_name,
"Save image".to_owned(),
png_bytes,
);
}
Err(err) => {
re_log::error!("{err}");
Expand Down
1 change: 1 addition & 0 deletions crates/viewer/re_viewer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ grpc = ["re_data_source/grpc", "dep:re_grpc_client"]
# Internal:
re_blueprint_tree.workspace = true
re_build_info.workspace = true
re_capabilities.workspace = true
re_chunk.workspace = true
re_chunk_store.workspace = true
re_component_ui.workspace = true
Expand Down
Loading
Loading