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

moka::future::Cache::get_or_insert_with() panics if previously inserting task aborted #59

Closed
niklasf opened this issue Dec 27, 2021 · 6 comments · Fixed by #60
Closed
Assignees
Labels
bug Something isn't working
Milestone

Comments

@niklasf
Copy link

niklasf commented Dec 27, 2021

When writing a web server, it appears that hyper::Server can abort tasks, if the requester has gone away. moka::future::Cache does not like that and panics. Is this bug? Or is there a recommended way to deal with it?

Minimized example:

[package]
name = "moka-future-bug"
version = "0.1.0"
edition = "2021"

[dependencies]
moka = { version = "0.6.2", features = ["future"] }
tokio = { version = "1.15.0", features = ["full"] }
use moka::future::Cache;
use std::time::Duration;

#[tokio::main]
async fn main() {
    let cache_a: Cache<(), ()> = Cache::new(1);

    let cache_b = cache_a.clone();

    let handle = tokio::task::spawn(async move {
        cache_b
            .get_or_insert_with((), async {
                tokio::time::sleep(Duration::from_millis(1000)).await;
            })
            .await;
    });

    tokio::time::sleep(Duration::from_millis(500)).await;
    handle.abort();

    cache_a.get_or_insert_with((), async {}).await; // panics!
}

Backtrace:

thread 'main' panicked at 'Too many retries. Tried to read the return value from the `init` \
                                future but failed 200 times. Maybe the `init` kept panicking?', /home/niklas/.cargo/registry/src/github.com-1ecc6299db9ec823/moka-0.6.2/src/future/value_initializer.rs:138:33
stack backtrace:
   0: rust_begin_unwind
             at /rustc/efec545293b9263be9edfb283a7aa66350b3acbf/library/std/src/panicking.rs:498:5
   1: core::panicking::panic_fmt
             at /rustc/efec545293b9263be9edfb283a7aa66350b3acbf/library/core/src/panicking.rs:107:14
   2: moka::future::value_initializer::ValueInitializer<K,V,S>::do_try_init::{{closure}}
             at /home/niklas/.cargo/registry/src/github.com-1ecc6299db9ec823/moka-0.6.2/src/future/value_initializer.rs:138:33
   3: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/efec545293b9263be9edfb283a7aa66350b3acbf/library/core/src/future/mod.rs:80:19
   4: moka::future::value_initializer::ValueInitializer<K,V,S>::init_or_read::{{closure}}
             at /home/niklas/.cargo/registry/src/github.com-1ecc6299db9ec823/moka-0.6.2/src/future/value_initializer.rs:53:9
   5: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/efec545293b9263be9edfb283a7aa66350b3acbf/library/core/src/future/mod.rs:80:19
   6: moka::future::cache::Cache<K,V,S>::get_or_insert_with_hash_and_fun::{{closure}}
             at /home/niklas/.cargo/registry/src/github.com-1ecc6299db9ec823/moka-0.6.2/src/future/cache.rs:621:15
   7: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/efec545293b9263be9edfb283a7aa66350b3acbf/library/core/src/future/mod.rs:80:19
   8: moka::future::cache::Cache<K,V,S>::get_or_insert_with::{{closure}}
             at /home/niklas/.cargo/registry/src/github.com-1ecc6299db9ec823/moka-0.6.2/src/future/cache.rs:363:9
   9: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/efec545293b9263be9edfb283a7aa66350b3acbf/library/core/src/future/mod.rs:80:19
  10: moka_future_bug::main::{{closure}}
             at ./src/main.rs:21:5
  11: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/efec545293b9263be9edfb283a7aa66350b3acbf/library/core/src/future/mod.rs:80:19
  12: tokio::park::thread::CachedParkThread::block_on::{{closure}}
             at /home/niklas/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.15.0/src/park/thread.rs:263:54
  13: tokio::coop::with_budget::{{closure}}
             at /home/niklas/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.15.0/src/coop.rs:102:9
  14: std::thread::local::LocalKey<T>::try_with
             at /rustc/efec545293b9263be9edfb283a7aa66350b3acbf/library/std/src/thread/local.rs:413:16
  15: std::thread::local::LocalKey<T>::with
             at /rustc/efec545293b9263be9edfb283a7aa66350b3acbf/library/std/src/thread/local.rs:389:9
  16: tokio::coop::with_budget
             at /home/niklas/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.15.0/src/coop.rs:95:5
  17: tokio::coop::budget
             at /home/niklas/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.15.0/src/coop.rs:72:5
  18: tokio::park::thread::CachedParkThread::block_on
             at /home/niklas/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.15.0/src/park/thread.rs:263:31
  19: tokio::runtime::enter::Enter::block_on
             at /home/niklas/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.15.0/src/runtime/enter.rs:151:13
  20: tokio::runtime::thread_pool::ThreadPool::block_on
             at /home/niklas/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.15.0/src/runtime/thread_pool/mod.rs:77:9
  21: tokio::runtime::Runtime::block_on
             at /home/niklas/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.15.0/src/runtime/mod.rs:463:43
  22: moka_future_bug::main
             at ./src/main.rs:21:5
  23: core::ops::function::FnOnce::call_once
             at /rustc/efec545293b9263be9edfb283a7aa66350b3acbf/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
@tatsuya6502 tatsuya6502 added the bug Something isn't working label Dec 28, 2021
@tatsuya6502 tatsuya6502 self-assigned this Dec 28, 2021
@tatsuya6502
Copy link
Member

Thank you for reporting. I confirmed the issue and I think it is a bug. I will investigate and fix it.

@barkanido
Copy link
Contributor

Hey @tatsuya6502 lmk if you think I could help here

@tatsuya6502
Copy link
Member

Hi @barkanido — Thank you for the offer. I will do this by myself because this one is a bit tricky to solve. I will need some experiments.

tatsuya6502 added a commit that referenced this issue Dec 28, 2021
…sk was aborted

Add `WaiterGuard` to `future::value_initializer` which will ensure that the
waiter will be removed when the enclosing future has been aborted.

Fixes #59.
@tatsuya6502 tatsuya6502 added this to the V0.6.3 milestone Dec 28, 2021
@tatsuya6502
Copy link
Member

This issue will be fixed by PR #60.

After merging the PR, I will start a release process for Moka v0.6.3. I will run a regular pre-release testing, which usually takes 2.5 hours, and If everything goes well, I will publish v0.6.3 to crates.io.

@tatsuya6502
Copy link
Member

Published v0.6.3 with the fix to crates.io.

@niklasf
Copy link
Author

niklasf commented Dec 28, 2021

Wow, thanks for the super fast tournaround on this!

Aside: It looks like in a web service it can still be a good idea to call get_or_*_insert_with from a new independent task, so clients cannot stall initialization by causing aborts - at least if other clients may be interested in the same key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants