OnceMap and singleflight leak an entry when a duplicate key's destructor panics
Maintainer thường phản hồi trong vòng 1 ngày
Chưa có ai nhận issue này.
Đánh giá
- Độ khó
- 4/5
- Thời gian dự kiến
- 1-2 ngày
- Mức phù hợp với người mới
- 55/100
Hướng nghiên cứu
Start with asyncband/src/once/once_map/mod.rs around get_or_insert and the pending-entry cleanup guard, then inspect asyncband/src/singleflight/mod.rs around get_or_insert and WorkCleanupGuard::new. Reproduce the panic cases described in the issue and trace what happens on pending and cached paths. Done means both APIs clean up correctly after a duplicate key's destructor panics, including cancellation during the drop.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
Summary
While working on #387 I found another way OnceMap can keep an empty entry. When compute or try_compute joins a computation that is already running for the key, the caller's own key is no longer needed and is dropped. If that destructor panics and the panic is caught, for example by tokio::spawn, the caller's reference to the entry leaks. Once the original computation is cancelled or fails, the empty entry stays in the map for good. singleflight::Group::work and try_work have the same problem.
Reproducer
On main (8febdc7, Rust 1.96.0) this prints OnceMap { len: 1, pending: 1 }, and the same program with singleflight::Group::work prints Group { in_flight: 1 }. Both are empty when the destructor doesn't panic.
use std::future::pending;
use std::hash::{Hash, Hasher};
use std::panic::{AssertUnwindSafe, catch_unwind};
use std::pin::pin;
use std::task::{Context, Waker};
use asyncband::once::OnceMap;
struct Key(&'static str, bool);
impl PartialEq for Key {
fn eq(&self, other: &Self) -> bool {
self.0 == other.0
}
}
impl Eq for Key {}
impl Hash for Key {
fn hash<H: Hasher>(&self, state: &mut H) {
self.0.hash(state);
}
}
impl Drop for Key {
fn drop(&mut self) {
if self.1 {
panic!("key destructor");
}
}
}
fn main() {
let mut cx = Context::from_waker(Waker::noop());
let map = OnceMap::<Key, i32>::new();
let mut original = Box::pin(map.compute(Key("k", false), async || pending().await));
assert!(original.as_mut().poll(&mut cx).is_pending());
let joined = catch_unwind(AssertUnwindSafe(|| {
let _ = pin!(map.compute(Key("k", true), async || pending().await)).poll(&mut cx);
}));
assert!(joined.is_err());
drop(original);
println!("{map:?}");
}
The leaked reference also keeps later values alive: if the key is computed again and then discarded, the value is never dropped. On a cached hit, the value clone that compute was about to return leaks instead.
Cause
The duplicate key is a parameter of get_or_insert (OnceMap, singleflight), so it is dropped when the function returns. If that destructor panics, Rust doesn't drop the value being returned (rust-lang/rust#47949, open since 2018), so the entry's Arc, or the cloned value on a cached hit, leaks. Even if it were dropped, no cleanup would run, because the guard is only created after get_or_insert returns. On OnceMap, making the drop explicit without moving the guard removes the permanent leak, but a cancellation that lands while the destructor runs still leaves an empty entry.
Possible fix
Drop the duplicate key explicitly, after creating the cleanup guard for a pending entry or cloning the value for a ready one. I tried this on OnceMap by having get_or_insert return either the value or the guard. Nothing leaks in the cases above, and the cached hit path still doesn't build a guard. singleflight would need the same change around WorkCleanupGuard::new.
Panicking destructors are unusual, so documenting the leak is also an option. I'd prefer to fix both, since the change is small. @tisonkun What do you think?
- Ngôn ngữ chính
- Rust
- Star
- 285
- Fork
- 49
- Merge trung bình
- 1 ngày 1 giờ
- Pull request đã merge (30 ngày)
- 33
Chuẩn bị môi trường
Dự án này không cung cấp dev container, Dockerfile hay hướng dẫn đóng góp, nên bạn cần tự thiết lập môi trường: hãy bắt đầu từ README và xem hướng dẫn đóng góp lần đầu của chúng tôi để biết các bước chung.
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của apache/asyncband
-
Độ khó 3/5 1-2 ngày Mức phù hợp với người mới 57/100
Maintainer thường phản hồi trong vòng 1 ngày
-
enhancement good first issue help wanted
Độ khó 5/5 Hơn một tuần Mức phù hợp với người mới 35/100
apache/asyncband#324 · 1 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Discuss shared broadcast receives without a Clone boundCó thể đã có người làm @orthur2 đã nhận 18 ngày trước. Đang mởenhancement question
Độ khó 5/5 Hơn một tuần Mức phù hợp với người mới 30/100
apache/asyncband#312 · 5 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
-
enhancement help wanted
Độ khó 5/5 Hơn một tuần Mức phù hợp với người mới 35/100
apache/asyncband#298 · 8 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 5/5 Hơn một tuần Mức phù hợp với người mới 30/100
apache/asyncband#272 · 2 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
Tất cả issue của apache/asyncband
Issue tương tự
-
bug
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
swc-project/swc-next#1165 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 73/100
Alive24/shea-symphony#574 ·
-
bug tests Windows
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
Maintainer thường phản hồi trong vòng 1 ngày
-
[ko] Translate /team into KoreanĐang mởdocumentation good first issue help wanted
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 74/100
dev-five-git/devup-ui#879 ·
Maintainer thường phản hồi trong vòng 11 ngày
-
bug
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 77/100
farion1231/cc-switch#9038 · 1 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày