-
Notifications
You must be signed in to change notification settings - Fork 54
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
zbus' blocking API causes panicks for async users #332
Comments
@zeenix Thanks for reporting this. Can you ask any of the users who reported this to give us either a link to their project if it's open source, or a minimal sample program that reproduces the problem? Feel free to just point them at this issue. |
@mwcampbell A little bit too high level of example, but it is how I run into this issue main.rs use eframe::egui;
#[tokio::main]
async fn main() -> Result<(), eframe::Error> {
eframe::run_native(
"My egui App",
eframe::NativeOptions {
..Default::default()
},
Box::new(|_cc| {
// This gives us image support:
Box::new(MyApp {})
}),
)
}
struct MyApp {}
impl eframe::App for MyApp {
fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
egui::CentralPanel::default().show(ctx, |_ui| {});
}
} Cargo.toml [package]
name = "tokio-zbus"
version = "0.1.0"
edition = "2021"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[dependencies]
# GUI
eframe = "0.24"
# Async framework
tokio = { version = "1", features = ["full"] }
# Need to access NetworkManager
zbus = { version = "3", default-features = false, features = ["tokio"] } Error
Dependency tree
|
More context in #227 So we use async zbus types everywhere now and we expose an
Until we figure out something else, the Unix adapter must not be used inside an async context. This should have been documented as part of #248, my mistake. Now it will be a pain to communicate this fact to projects like egui... Once we bump our MSRV to 1.75, maybe we'll be able to expose a fully async API. |
woa! That's internal API. I only exposed that for our tests (and macros I think). I suggest not using API that's marked as
That's a good choice.
👍
Mentioning this in the crate docs and README would go a long way, I think.
That would be the best solution. |
Is the use of Having |
@ids1024 The behavior changed in accesskit_unix 0.5.0 and accesskit_winit 0.14.0. |
I used We initially wanted to not bring our own executor and try to integrate with whatever the user already had. Since we basically dropped this idea for tokio, we might as well do the same for async-std. Exposing an async API feels nice, but since AccessKit adapters have to integrate with the event loop, this might not help much. That is, until an async windowing crate comes along. |
Use of internal API is a strange solution to the problem tbh but your project, your way. :) Just keep in mind, that this API can change or even removed in any release (even a patch release, if needed).
I see that you provide
It's not only nice but also something I'd expect from any API that does I/O (especially IPC) as a user these days.
I'm not sure that's necessarily required. There are at least 2 options here:
|
This is the route I have already taken. In fact all the calls to zbus happen inside a single task since latest accesskit_unix version. So the best solution here would probably be to make the code running in the main/UI thread completely blocking, and spawn our own executor no matter which feature is selected. This way we don't care if the adapter is used inside an async context. |
@DataTriny Yeah, that sounds right. And the code in the main thread should use something like |
I'm getting users (primarily tokio) coming to me about strange panics like these even though they're not using zbus directly:
Turns out accesskit is using the blocking API of zbus and of course, that's going to get you in trouble if you call them from an async context with most runtimes. tokio at least gives a hint so people know who to contact but other runtimes may just hang.
zbus is primarily async since version 2 and the blocking APIs are only there for folks who just want to quickly try the API or folks who are 100% sure async is not something they'll want to use (which is typically application developers).
For libraries, I highly recommend using the async API (providing blocking wrappers where appropriate). If blocking API is really what you want, I recommend incompatibility with async be well-advertised in the docs. Currently, crate level docs don't even exist. They should document this very important fact IMO.
The text was updated successfully, but these errors were encountered: