hwchen / keyring-rs

Cross-platform library and utility to manage passwords
Apache License 2.0
450 stars 49 forks source link

Panic if called within a tokio runtime on Linux #132

Closed connor4312 closed 1 year ago

connor4312 commented 1 year ago

Calling .get_password() from within a Tokio runtime panics on Linux, using the linux-secret-service-rt-tokio-crypto-openssl feature.

thread 'main' panicked at '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.', /home/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/zbus-3.13.1/src/utils.rs:47:14
stack backtrace:
   0: rust_begin_unwind
             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panicking.rs:579:5
   1: core::panicking::panic_fmt
             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/panicking.rs:64:14
   2: tokio::runtime::context::enter_runtime
             at /home/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.28.2/src/runtime/context.rs:183:9
   3: tokio::runtime::scheduler::current_thread::CurrentThread::block_on
             at /home/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.28.2/src/runtime/scheduler/current_thread.rs:146:25
   4: tokio::runtime::runtime::Runtime::block_on
             at /home/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.28.2/src/runtime/runtime.rs:302:47
   5: zbus::utils::block_on
             at /home/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/zbus-3.13.1/src/utils.rs:47:5
   6: zbus::blocking::connection::Connection::session
             at /home/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/zbus-3.13.1/src/blocking/connection.rs:33:9
   7: secret_service::blocking::SecretService::connect
             at /home/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/secret-service-3.0.1/src/blocking/mod.rs:49:20
   8: keyring::secret_service::SsCredential::map_matching_items
             at /home/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/keyring-2.0.3/src/secret_service.rs:239:18
   9: <keyring::secret_service::SsCredential as keyring::credential::CredentialApi>::get_password
             at /home/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/keyring-2.0.3/src/secret_service.rs:133:38
  10: keyring::Entry::get_password
             at /home/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/keyring-2.0.3/src/lib.rs:244:9

One workaround, which I'll probably take on my end, is to spawn the operation on another thread. A better but viral change would be exposing async methods to avoid zbus' blocking API.

https://github.com/microsoft/vscode/issues/184792

brotskydotcom commented 1 year ago

I hate to say this, but with the way the blocking interface in secret-service works, this is pretty much "as designed." I will take a note to mention this issue (and the workaround of using a separate thread for all keyring calls) in the docs for the secret-service keystore. Feel free to submit a PR for a suggested doc change.

@landhb and I have talked about maybe building a "native" dbus keystore (based on the dbus crate) to avoid this problem.

I've never really considered exposing an async interface to keyring because so many of the platform native keystores are synchronous. It feels like it would be disingenuous to claim the API was async when every call to every keystore other than secret-service would either block the runtime or spin up a separate thread to do the work. We might as well force the async client to spin up that thread (like we do now with secret service), so the client has control.

connor4312 commented 1 year ago

Yea, that's understandable. It's just unfortunate that this is a hidden problem that only pops up at runtime on Linux. Maybe something that could be done in secret-service, if using tokio as a runtime, is calling Handle::try_current() and then automatically running on another thread if that is Ok.

brotskydotcom commented 1 year ago

Feel free to file a secret-service issue and PR if you think you have an idea about how this could be done safely.