Stream: t-compiler/wg-prioritization/alerts

Topic: #82533 No Panic Safety in `extend_from_within`

triagebot (Feb 26 2021 at 04:29, on Zulip):

@WG-prioritization/alerts issue #82533 has been requested for prioritization.

Procedure

apiraino (Feb 26 2021 at 08:16, on Zulip):

hm ... what's the context of this issue? What's the actionable if (to my understanding by reading Joshua's comment) there's no memory leak?

hyd-dev (Feb 26 2021 at 10:19, on Zulip):

I guess Joshua Nelson meant this is "just" a memory leak, it's fully safe.

hyd-dev (Feb 26 2021 at 10:23, on Zulip):

Also IMO it's still called "panic safety" (or at least "unwind safety") even it's fully safe: https://doc.rust-lang.org/nightly/std/panic/trait.UnwindSafe.html#what-is-unwind-safety

apiraino (Feb 26 2021 at 10:36, on Zulip):

oh now I see, thank you!

Joshua Nelson (Feb 26 2021 at 14:39, on Zulip):

hyd-dev said:

Also IMO it's still called "panic safety" (or at least "unwind safety") even it's fully safe: https://doc.rust-lang.org/nightly/std/panic/trait.UnwindSafe.html#what-is-unwind-safety

Hmm, I don't think I'd call leaking memory a "broken logic invariant" - from what I can tell it's fine to catch the unwind and use the Vec after a panic

hyd-dev (Feb 26 2021 at 15:08, on Zulip):

Joshua Nelson said:

Hmm, I don't think I'd call leaking memory a "broken logic invariant" - from what I can tell it's fine to catch the unwind and use the Vec after a panic

Indeed... OTOH "resource leaks" are not considered exception safe in C++: https://en.cppreference.com/w/cpp/language/exceptions#Exception_safety

But yes, although "panics in Rust are currently implemented essentially as a C++ exception under the hood", memory leak is mentioned in neither the documentation nor the RFC.

Maybe I just hate memory leaks too much. :losing_money:

Joshua Nelson (Feb 26 2021 at 15:09, on Zulip):

yeah, I don't think C++ is super relevant to Rust's idea of unwind safety

Hameer Abbasi (Feb 26 2021 at 15:53, on Zulip):

I'd still label this P-high. It may be safe, but we don't want to blow up memory in contexts where panics are common.

Joshua Nelson (Feb 26 2021 at 16:03, on Zulip):

I don't think panics should be common :shrug: and if they are I'd expect a lot more things to break than just leaked memory

Hameer Abbasi (Feb 26 2021 at 16:28, on Zulip):

Then maybe we should document this, at least, in very big bold letters, that using panics for anything other than exit situations breaks things. Use std::Result instead.

Joshua Nelson (Feb 26 2021 at 16:36, on Zulip):

Sorry, I'm not explaining myself very well. I think catch_unwind is ok to use and won't inherently break things, just that in practice people don't consider it when writing libraries and so it doesn't get tested very well. I do think we should fix this if possible, I just don't think it's high priority.

Hameer Abbasi (Feb 26 2021 at 16:39, on Zulip):

P-medium, then? P-low is for things unlikely to get fixed, historically.

Camelid (Feb 26 2021 at 19:51, on Zulip):

P-medium seems fine to me.

Hameer Abbasi (Feb 26 2021 at 22:20, on Zulip):

I assigned a Prio, it went to this thread due to the issue title modification.

Last update: Apr 11 2021 at 17:45UTC