neqo icon indicating copy to clipboard operation
neqo copied to clipboard

primary_path.borrow_mut() already borrowed in loss_recovery::timeout

Open valenting opened this issue 1 year ago • 3 comments

https://crash-stats.mozilla.org/report/index/42db51ff-8988-472e-a6ef-1bb380240108 crashes on https://hg.mozilla.org/releases/mozilla-esr115/file/aa9f02961b2bbb92e17fea5d6b8fd14c097baf72/third_party/rust/neqo-transport/src/recovery.rs#l943

https://searchfox.org/mozilla-central/rev/734887cfb193b5697e59a857d394e8d4245996db/third_party/rust/neqo-transport/src/recovery.rs#940-944

primary_path.borrow_mut().on_packets_lost(
    space.largest_acked_sent_time,
    space.space(),
    &lost_packets[first..],
);

Report in bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=1874584

valenting avatar Jan 23 '24 16:01 valenting

I'm looking at the code in question, but with my limited Rust-fu I can't tell why this error occurs. Shouldn't the borrow_mut() lifetime end with the loop block?

larseggert avatar Apr 10 '24 15:04 larseggert

Rust has non-lexical lifetimes, which means that in some cases a borrow might not last as long as the block in which it is found, though there is a guarantee that borrows are released at the end of a block.

Looking at the code in the immediate vicinity, the calls to borrow() or borrow_mut() on primary_path all end the borrow immediately, until the call to maybe_fire_pto(), which is passed a reference to the path RTT object. I'd look further afield. Maybe we're holding a borrow further up the stack.

martinthomson avatar Apr 16 '24 00:04 martinthomson

@valenting have we recently seen more crash reports like this?

larseggert avatar May 31 '24 05:05 larseggert

I'm going to close this, since we removed a lot of related unwraps.

larseggert avatar Sep 13 '24 13:09 larseggert