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

zbus' blocking API causes panicks for async users #332

Closed
zeenix opened this issue Jan 5, 2024 · 10 comments · Fixed by #337
Closed

zbus' blocking API causes panicks for async users #332

zeenix opened this issue Jan 5, 2024 · 10 comments · Fixed by #337
Assignees

Comments

@zeenix
Copy link

zeenix commented Jan 5, 2024

I'm getting users (primarily tokio) coming to me about strange panics like these even though they're not using zbus directly:

thread 'main' panicked at [...]/zbus-3.14.1/src/utils.rs:47:14:
Cannot start a runtime from within a runtime. This happens because a function (like `block_on`) attempted to block the current thread while the thread is being used to drive asynchronous tasks.

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.

@zeenix zeenix changed the title zbus's blocking API causes panicks for async users zbus' blocking API causes panicks for async users Jan 5, 2024
@mwcampbell
Copy link
Contributor

@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.

@olehkres
Copy link

olehkres commented Jan 5, 2024

@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

    Finished dev [unoptimized + debuginfo] target(s) in 0.16s
     Running `target/debug/tokio-zbus`
thread 'main' panicked at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zbus-3.14.1/src/utils.rs:47:14:
Cannot start a runtime from within a runtime. This happens because a function (like `block_on`) attempted to block the current thread while the thread is being used to drive asynchronous tasks.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[user@localhost tokio-zbus]$ RUST_BACKTRACE=1 cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
     Running `target/debug/tokio-zbus`
thread 'main' panicked at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zbus-3.14.1/src/utils.rs:47:14:
Cannot start a runtime from within a runtime. This happens because a function (like `block_on`) attempted to block the current thread while the thread is being used to drive asynchronous tasks.
stack backtrace:
   0: rust_begin_unwind
             at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/panicking.rs:72:14
   2: tokio::runtime::context::runtime::enter_runtime
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.35.1/src/runtime/context/runtime.rs:68:5
   3: tokio::runtime::scheduler::current_thread::CurrentThread::block_on
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.35.1/src/runtime/scheduler/current_thread/mod.rs:167:9
   4: tokio::runtime::runtime::Runtime::block_on
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.35.1/src/runtime/runtime.rs:348:47
   5: zbus::utils::block_on
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zbus-3.14.1/src/utils.rs:47:5
   6: accesskit_unix::util::block_on
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/accesskit_unix-0.6.2/src/util.rs:13:5
   7: accesskit_unix::adapter::Adapter::new
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/accesskit_unix-0.6.2/src/adapter.rs:219:29
   8: accesskit_winit::platform_impl::platform::Adapter::new
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/accesskit_winit-0.15.0/src/platform_impl/unix.rs:21:23
   9: accesskit_winit::Adapter::with_action_handler
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/accesskit_winit-0.15.0/src/lib.rs:73:23
  10: accesskit_winit::Adapter::new
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/accesskit_winit-0.15.0/src/lib.rs:56:9
  11: egui_winit::State::init_accesskit
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/egui-winit-0.24.1/src/lib.rs:161:31
  12: eframe::native::epi_integration::EpiIntegration::init_accesskit
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/eframe-0.24.1/src/native/epi_integration.rs:214:9
  13: eframe::native::glow_integration::GlowWinitApp::init_run_state
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/eframe-0.24.1/src/native/glow_integration.rs:262:17
  14: <eframe::native::glow_integration::GlowWinitApp as eframe::native::winit_integration::WinitApp>::on_event
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/eframe-0.24.1/src/native/glow_integration.rs:418:21
  15: eframe::native::run::run_and_return::{{closure}}
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/eframe-0.24.1/src/native/run.rs:123:28
  16: winit::platform_impl::platform::sticky_exit_callback
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/winit-0.28.7/src/platform_impl/linux/mod.rs:884:9
  17: winit::platform_impl::platform::x11::EventLoop<T>::run_return::single_iteration
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/winit-0.28.7/src/platform_impl/linux/x11/mod.rs:334:17
  18: winit::platform_impl::platform::x11::EventLoop<T>::run_return
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/winit-0.28.7/src/platform_impl/linux/x11/mod.rs:443:31
  19: winit::platform_impl::platform::EventLoop<T>::run_return
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/winit-0.28.7/src/platform_impl/linux/mod.rs:785:56
  20: <winit::event_loop::EventLoop<T> as winit::platform::run_return::EventLoopExtRunReturn>::run_return
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/winit-0.28.7/src/platform/run_return.rs:51:9
  21: eframe::native::run::run_and_return
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/eframe-0.24.1/src/native/run.rs:79:5
  22: eframe::native::run::run_glow::{{closure}}
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/eframe-0.24.1/src/native/run.rs:372:13
  23: eframe::native::run::with_event_loop::{{closure}}
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/eframe-0.24.1/src/native/run.rs:61:9
  24: std::thread::local::LocalKey<T>::try_with
             at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/thread/local.rs:270:16
  25: std::thread::local::LocalKey<T>::with
             at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/thread/local.rs:246:9
  26: eframe::native::run::with_event_loop
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/eframe-0.24.1/src/native/run.rs:55:5
  27: eframe::native::run::run_glow
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/eframe-0.24.1/src/native/run.rs:370:16
  28: eframe::run_native
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/eframe-0.24.1/src/lib.rs:249:13
  29: tokio_zbus::main::{{closure}}
             at ./src/main.rs:5:5
  30: tokio::runtime::park::CachedParkThread::block_on::{{closure}}
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.35.1/src/runtime/park.rs:282:63
  31: tokio::runtime::coop::with_budget
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.35.1/src/runtime/coop.rs:107:5
  32: tokio::runtime::coop::budget
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.35.1/src/runtime/coop.rs:73:5
  33: tokio::runtime::park::CachedParkThread::block_on
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.35.1/src/runtime/park.rs:282:31
  34: tokio::runtime::context::blocking::BlockingRegionGuard::block_on
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.35.1/src/runtime/context/blocking.rs:66:9
  35: tokio::runtime::scheduler::multi_thread::MultiThread::block_on::{{closure}}
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.35.1/src/runtime/scheduler/multi_thread/mod.rs:87:13
  36: tokio::runtime::context::runtime::enter_runtime
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.35.1/src/runtime/context/runtime.rs:65:16
  37: tokio::runtime::scheduler::multi_thread::MultiThread::block_on
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.35.1/src/runtime/scheduler/multi_thread/mod.rs:86:9
  38: tokio::runtime::runtime::Runtime::block_on
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.35.1/src/runtime/runtime.rs:350:45
  39: tokio_zbus::main
             at ./src/main.rs:5:5
  40: core::ops::function::FnOnce::call_once
             at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Dependency tree

cargo tree -i zbus
zbus v3.14.1
├── accesskit_unix v0.6.2
│   └── accesskit_winit v0.15.0
│       └── egui-winit v0.24.1
│           └── eframe v0.24.1
│               └── project v0.1.0 (...)
├── atspi-common v0.3.0
│   ├── atspi v0.19.0
│   │   └── accesskit_unix v0.6.2 (*)
│   ├── atspi-connection v0.3.0
│   │   └── atspi v0.19.0 (*)
│   └── atspi-proxies v0.3.0
│       ├── atspi v0.19.0 (*)
│       └── atspi-connection v0.3.0 (*)
├── atspi-connection v0.3.0 (*)
├── atspi-proxies v0.3.0 (*)
└── project v0.1.0 (...)

@DataTriny
Copy link
Member

More context in #227

So we use async zbus types everywhere now and we expose an async-std and a tokio feature. But since we don't expose an async API, we have to run everything inside:

  • zbus::block_on if the tokio feature is not enabled,
  • a single-threaded tokio executor otherwise.

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.

@zeenix
Copy link
Author

zeenix commented Jan 5, 2024

  • zbus::block_on if the tokio feature is not enabled,

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 #[doc(hidden)]. That's almost always a bad idea in general.

a single-threaded tokio executor otherwise.

That's a good choice.

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...

Mentioning this in the crate docs and README would go a long way, I think.

Once we bump our MSRV to 1.75, maybe we'll be able to expose a fully async API.

That would be the best solution.

@ids1024
Copy link

ids1024 commented Jan 5, 2024

Is the use of zbus::block_on in accesskit recent? In dbus2/zbus#526 I mentioned an issue with there is no reactor running, must be called from the context of a Tokio 1.x runtime' when the tokio feature of zbus but not accesskit is enabled. Not seeing that if I try it now, it seems.

Having zbus::block_on public makes sense for this sort of thing, since it is guaranteed to work with zbus futures, while something like futures::executor::block_on would panic if the tokio feature of zbus were enabled.

@DataTriny
Copy link
Member

@ids1024 The behavior changed in accesskit_unix 0.5.0 and accesskit_winit 0.14.0.

@DataTriny
Copy link
Member

@zeenix

I used zbus::block_on even though it is internal stuff because we were really concerned with the size of the dependency tree, and it was a convenient helper.

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.

@zeenix
Copy link
Author

zeenix commented Jan 6, 2024

I used zbus::block_on even though it is internal stuff because we were really concerned with the size of the dependency tree, and it was a convenient helper.

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).

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

I see that you provide tokio and async-io features. Why not use block_on from either of them, based on the enabled feature (just like zbus does)?

Exposing an async API feels nice,

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.

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.

I'm not sure that's necessarily required. There are at least 2 options here:

  • Make use of tokio::task::spawn_blocking or blocking::unblock (in case of async-io) to turn blocking I/O calls into async calls.
  • Use the typical solution of splitting async and blocking API use in separate threads, communicating with channels between them.

@DataTriny
Copy link
Member

@zeenix

Use the typical solution of splitting async and blocking API use in separate threads, communicating with channels between them.

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.

@mwcampbell
Copy link
Contributor

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 futures_lite::future::block_on, so it's not dependent on async-io or tokio.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants