search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.33k stars 12.58k forks source link

Fix memory leaks in doctests? #126067

Closed RalfJung closed 1 month ago

RalfJung commented 4 months ago

Currently a bunch of our doctests have memory leaks. For that reason, we have to disable Miri's leak checker when running doctests.

To reproduce:

MIRIFLAGS="-Zmiri-disable-isolation" ./x.py miri library/core library/alloc --doc
MIRIFLAGS="-Zmiri-disable-isolation" ./x.py miri library/std --doc -- --skip fs:: --skip net:: --skip process:: --skip sys::pal::

@rust-lang/libs how do you feel about that -- is it okay for the tests to leak, or should we fix that (by adding hidden code that deallocates things again)? Currently we don't have a good way to disable the leak checker on a per-test basis so there is a risk of accidental leaks.

RalfJung commented 4 months ago

One (I think somewhat typical) example of a leaky test is this: a Vec is created but remains in a MaybeUninit so it is never dropped.

ChrisDenton commented 3 months ago

I've nominated for libs-api to ensure this gets discussed but feel free to un-nominate if a decision is made before a meeting.

the8472 commented 3 months ago

One (I think somewhat typical) example of a leaky test is this: a Vec is created but remains in a MaybeUninit so it is never dropped.

Was that meant as an example of one that takes extra steps to unleak it? Since it calls assume_init which makes the vec droppable again.

The Vec::leak doctest is an example that leaks.

RalfJung commented 3 months ago

Ah, I picked the wrong test indeed, sorry. Here is an example that leaks.

Amanieu commented 3 months ago

We discussed this in the libs meeting today. We would prefer to have miri check for leaks in doctests and handle the currently leaking tests by either:

RalfJung commented 3 months ago

Adding a doctest attribute such as no_leak_check which disables leak checking just for that test.

That could be done if rustdoc allowed passing flags to Miri... but the could be only for Miri, not rustc, so not sure how to best do this. Alternatively, https://github.com/rust-lang/miri/issues/3670 would help.

RalfJung commented 1 month ago

This got fixed by https://github.com/rust-lang/rust/pull/127446 :)