Drop panic hook after running tests by Duckilicious · Pull Request #119224 · rust-lang/rust

Duckilicious · 2023-12-22T20:37:46Z

Issue: #119223
Previously we left the panic hook we allocated
on main termination. Doing so makes Valgrind
report it as a reachable unfreed block.
In order to fix that use panic::take_hook() before examining test results.

Example backtrace:

==146594== 16 bytes in 1 blocks are still reachable in loss record 1 of 1
==146594==    at 0x4A390C5: malloc (vg_replace_malloc.c:442)
==146594==    by 0x151336: alloc (alloc.rs:98)
==146594==    by 0x151336: alloc_impl (alloc.rs:181)
==146594==    by 0x151336: allocate (alloc.rs:241)
==146594==    by 0x151336: exchange_malloc (alloc.rs:330)
==146594==    by 0x151336: new<test::test_main::{closure_env#0}> (boxed.rs:217)
==146594==    by 0x151336: test::test_main (lib.rs:124)
==146594==    by 0x1522F9: test::test_main_static (lib.rs:160)
==146594==    by 0x11E102: reachable_block_with_cargo_test::main (lib.rs:1)
==146594==    by 0x11EABA: core::ops::function::FnOnce::call_once (function.rs:250)
==146594==    by 0x11E76D: std::sys_common::backtrace::__rust_begin_short_backtrace (backtrace.rs:154)
==146594==    by 0x11DFC0: std::rt::lang_start::{{closure}} (rt.rs:166)
==146594==    by 0x177D3A: call_once<(), (dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe)> (function.rs:284)
==146594==    by 0x177D3A: do_call<&(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe), i32> (panicking.rs:504)
==146594==    by 0x177D3A: try<i32, &(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe)> (panicking.rs:468)
==146594==    by 0x177D3A: catch_unwind<&(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe), i32> (panic.rs:142)
==146594==    by 0x177D3A: {closure#2} (rt.rs:148)
==146594==    by 0x177D3A: do_call<std::rt::lang_start_internal::{closure_env#2}, isize> (panicking.rs:504)
==146594==    by 0x177D3A: try<isize, std::rt::lang_start_internal::{closure_env#2}> (panicking.rs:468)
==146594==    by 0x177D3A: catch_unwind<std::rt::lang_start_internal::{closure_env#2}, isize> (panic.rs:142)
==146594==    by 0x177D3A: std::rt::lang_start_internal (rt.rs:148)
==146594==    by 0x11DF99: std::rt::lang_start (rt.rs:165)

rustbot · 2023-12-22T20:37:54Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @m-ou-se (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

library/test/src/lib.rs

Previously we left the panic hook we allocated on main termination. Doing so makes Valgrind report it as a reachable unfreed block. In order to fix that use `panic::take_hook()` before examining test results. Example backtrace: ``` ==146594== 16 bytes in 1 blocks are still reachable in loss record 1 of 1 ==146594== at 0x4A390C5: malloc (vg_replace_malloc.c:442) ==146594== by 0x151336: alloc (alloc.rs:98) ==146594== by 0x151336: alloc_impl (alloc.rs:181) ==146594== by 0x151336: allocate (alloc.rs:241) ==146594== by 0x151336: exchange_malloc (alloc.rs:330) ==146594== by 0x151336: new<test::test_main::{closure_env#0}> (boxed.rs:217) ==146594== by 0x151336: test::test_main (lib.rs:124) ==146594== by 0x1522F9: test::test_main_static (lib.rs:160) ==146594== by 0x11E102: reachable_block_with_cargo_test::main (lib.rs:1) ==146594== by 0x11EABA: core::ops::function::FnOnce::call_once (function.rs:250) ==146594== by 0x11E76D: std::sys_common::backtrace::__rust_begin_short_backtrace (backtrace.rs:154) ==146594== by 0x11DFC0: std::rt::lang_start::{{closure}} (rt.rs:166) ==146594== by 0x177D3A: call_once<(), (dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe)> (function.rs:284) ==146594== by 0x177D3A: do_call<&(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe), i32> (panicking.rs:504) ==146594== by 0x177D3A: try<i32, &(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe)> (panicking.rs:468) ==146594== by 0x177D3A: catch_unwind<&(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe), i32> (panic.rs:142) ==146594== by 0x177D3A: {closure#2} (rt.rs:148) ==146594== by 0x177D3A: do_call<std::rt::lang_start_internal::{closure_env#2}, isize> (panicking.rs:504) ==146594== by 0x177D3A: try<isize, std::rt::lang_start_internal::{closure_env#2}> (panicking.rs:468) ==146594== by 0x177D3A: catch_unwind<std::rt::lang_start_internal::{closure_env#2}, isize> (panic.rs:142) ==146594== by 0x177D3A: std::rt::lang_start_internal (rt.rs:148) ==146594== by 0x11DF99: std::rt::lang_start (rt.rs:165) ``` Signed-off-by: Tal Gelbard <talgelbard1@gmail.com>

Duckilicious · 2024-04-06T15:59:40Z

Before:

valgrind target/debug/deps/reachable_block_with_cargo_test-049d9bdb11a1a464
==80325== Memcheck, a memory error detector
==80325== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==80325== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==80325== Command: target/debug/deps/reachable_block_with_cargo_test-049d9bdb11a1a464
==80325==

running 1 test
test tests::it_works ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.09s

==80325==
==80325== HEAP SUMMARY:
==80325==     in use at exit: 16 bytes in 1 blocks
==80325==   total heap usage: 446 allocs, 445 frees, 61,882 bytes allocated
==80325==
==80325== LEAK SUMMARY:
==80325==    definitely lost: 0 bytes in 0 blocks
==80325==    indirectly lost: 0 bytes in 0 blocks
==80325==      possibly lost: 0 bytes in 0 blocks
==80325==    still reachable: 16 bytes in 1 blocks
==80325==         suppressed: 0 bytes in 0 blocks
==80325== Rerun with --leak-check=full to see details of leaked memory
==80325==
==80325== For lists of detected and suppressed errors, rerun with: -s
==80325== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

After:

❯ valgrind target/debug/deps/reachable_block_with_cargo_test-47711fac179ab84c
==80182== Memcheck, a memory error detector
==80182== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==80182== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==80182== Command: target/debug/deps/reachable_block_with_cargo_test-47711fac179ab84c
==80182==

running 1 test
test tests::it_works ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.11s

==80182==
==80182== HEAP SUMMARY:
==80182==     in use at exit: 0 bytes in 0 blocks
==80182==   total heap usage: 445 allocs, 445 frees, 61,893 bytes allocated
==80182==
==80182== All heap blocks were freed -- no leaks are possible
==80182==
==80182== For lists of detected and suppressed errors, rerun with: -s
==80182== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

cuviper · 2024-04-06T16:08:26Z

Thanks!

@bors r+ rollup

bors · 2024-04-06T16:08:29Z

📌 Commit 92ebf60 has been approved by cuviper

It is now in the queue for this repository.

…, r=cuviper Drop panic hook after running tests Issue: rust-lang#119223 Previously we left the panic hook we allocated on main termination. Doing so makes Valgrind report it as a reachable unfreed block. In order to fix that use `panic::take_hook()` before examining test results. Example backtrace: ``` ==146594== 16 bytes in 1 blocks are still reachable in loss record 1 of 1 ==146594== at 0x4A390C5: malloc (vg_replace_malloc.c:442) ==146594== by 0x151336: alloc (alloc.rs:98) ==146594== by 0x151336: alloc_impl (alloc.rs:181) ==146594== by 0x151336: allocate (alloc.rs:241) ==146594== by 0x151336: exchange_malloc (alloc.rs:330) ==146594== by 0x151336: new<test::test_main::{closure_env#0}> (boxed.rs:217) ==146594== by 0x151336: test::test_main (lib.rs:124) ==146594== by 0x1522F9: test::test_main_static (lib.rs:160) ==146594== by 0x11E102: reachable_block_with_cargo_test::main (lib.rs:1) ==146594== by 0x11EABA: core::ops::function::FnOnce::call_once (function.rs:250) ==146594== by 0x11E76D: std::sys_common::backtrace::__rust_begin_short_backtrace (backtrace.rs:154) ==146594== by 0x11DFC0: std::rt::lang_start::{{closure}} (rt.rs:166) ==146594== by 0x177D3A: call_once<(), (dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe)> (function.rs:284) ==146594== by 0x177D3A: do_call<&(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe), i32> (panicking.rs:504) ==146594== by 0x177D3A: try<i32, &(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe)> (panicking.rs:468) ==146594== by 0x177D3A: catch_unwind<&(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe), i32> (panic.rs:142) ==146594== by 0x177D3A: {closure#2} (rt.rs:148) ==146594== by 0x177D3A: do_call<std::rt::lang_start_internal::{closure_env#2}, isize> (panicking.rs:504) ==146594== by 0x177D3A: try<isize, std::rt::lang_start_internal::{closure_env#2}> (panicking.rs:468) ==146594== by 0x177D3A: catch_unwind<std::rt::lang_start_internal::{closure_env#2}, isize> (panic.rs:142) ==146594== by 0x177D3A: std::rt::lang_start_internal (rt.rs:148) ==146594== by 0x11DF99: std::rt::lang_start (rt.rs:165) ```

…llaumeGomez Rollup of 4 pull requests Successful merges: - rust-lang#119224 (Drop panic hook after running tests) - rust-lang#123411 (Put checks that detect UB under their own flag below debug_assertions) - rust-lang#123446 (Fix incorrect 'llvm_target' value used on watchOS target) - rust-lang#123516 (Do not ICE on field access check on expr with `ty::Error`) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#119224 (Drop panic hook after running tests) - rust-lang#123411 (Put checks that detect UB under their own flag below debug_assertions) - rust-lang#123516 (Do not ICE on field access check on expr with `ty::Error`) - rust-lang#123522 (Stabilize const Atomic*::into_inner) - rust-lang#123559 (Add a debug asserts call to match_projection_projections to ensure invariant) - rust-lang#123563 (Rewrite `version` test run-make as an UI test) Failed merges: - rust-lang#123569 (Move some tests) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#119224 - Duckilicious:test_main_memory_leak, r=cuviper Drop panic hook after running tests Issue: rust-lang#119223 Previously we left the panic hook we allocated on main termination. Doing so makes Valgrind report it as a reachable unfreed block. In order to fix that use `panic::take_hook()` before examining test results. Example backtrace: ``` ==146594== 16 bytes in 1 blocks are still reachable in loss record 1 of 1 ==146594== at 0x4A390C5: malloc (vg_replace_malloc.c:442) ==146594== by 0x151336: alloc (alloc.rs:98) ==146594== by 0x151336: alloc_impl (alloc.rs:181) ==146594== by 0x151336: allocate (alloc.rs:241) ==146594== by 0x151336: exchange_malloc (alloc.rs:330) ==146594== by 0x151336: new<test::test_main::{closure_env#0}> (boxed.rs:217) ==146594== by 0x151336: test::test_main (lib.rs:124) ==146594== by 0x1522F9: test::test_main_static (lib.rs:160) ==146594== by 0x11E102: reachable_block_with_cargo_test::main (lib.rs:1) ==146594== by 0x11EABA: core::ops::function::FnOnce::call_once (function.rs:250) ==146594== by 0x11E76D: std::sys_common::backtrace::__rust_begin_short_backtrace (backtrace.rs:154) ==146594== by 0x11DFC0: std::rt::lang_start::{{closure}} (rt.rs:166) ==146594== by 0x177D3A: call_once<(), (dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe)> (function.rs:284) ==146594== by 0x177D3A: do_call<&(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe), i32> (panicking.rs:504) ==146594== by 0x177D3A: try<i32, &(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe)> (panicking.rs:468) ==146594== by 0x177D3A: catch_unwind<&(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe), i32> (panic.rs:142) ==146594== by 0x177D3A: {closure#2} (rt.rs:148) ==146594== by 0x177D3A: do_call<std::rt::lang_start_internal::{closure_env#2}, isize> (panicking.rs:504) ==146594== by 0x177D3A: try<isize, std::rt::lang_start_internal::{closure_env#2}> (panicking.rs:468) ==146594== by 0x177D3A: catch_unwind<std::rt::lang_start_internal::{closure_env#2}, isize> (panic.rs:142) ==146594== by 0x177D3A: std::rt::lang_start_internal (rt.rs:148) ==146594== by 0x11DF99: std::rt::lang_start (rt.rs:165) ```

rustbot assigned m-ou-se Dec 22, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 22, 2023

cuviper reviewed Feb 14, 2024

View reviewed changes

library/test/src/lib.rs Show resolved Hide resolved

cuviper assigned cuviper and unassigned m-ou-se Feb 14, 2024

Amanieu added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 21, 2024

rustbot added the has-merge-commits PR has merge commits, merge with caution. label Apr 6, 2024

This comment was marked as resolved.

Sign in to view

Duckilicious force-pushed the test_main_memory_leak branch from fbfda7d to 92ebf60 Compare April 6, 2024 15:57

rustbot removed the has-merge-commits PR has merge commits, merge with caution. label Apr 6, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 6, 2024

GuillaumeGomez mentioned this pull request Apr 6, 2024

Rollup of 4 pull requests #123565

Closed

matthiaskrgr mentioned this pull request Apr 6, 2024

Rollup of 6 pull requests #123576

Merged

bors merged commit 0ea4270 into rust-lang:master Apr 7, 2024

rustbot added this to the 1.79.0 milestone Apr 7, 2024

This was referenced Aug 2, 2024

Update to rust 1.80.0 servo/servo#32896

Merged

Investigate panic while dropping OsIpcReceiverSet servo/servo#32912

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Drop panic hook after running tests#119224

Drop panic hook after running tests#119224
bors merged 1 commit intorust-lang:masterfrom
Duckilicious:test_main_memory_leak

Duckilicious commented Dec 22, 2023

Uh oh!

rustbot commented Dec 22, 2023

Uh oh!

Uh oh!

This comment was marked as resolved.

Duckilicious commented Apr 6, 2024

Uh oh!

cuviper commented Apr 6, 2024

Uh oh!

bors commented Apr 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

Duckilicious commented Dec 22, 2023

Uh oh!

rustbot commented Dec 22, 2023

Uh oh!

Uh oh!

This comment was marked as resolved.

Duckilicious commented Apr 6, 2024

Uh oh!

cuviper commented Apr 6, 2024

Uh oh!

bors commented Apr 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants