Skip to content

Commit

Permalink
Rollup merge of #111875 - WaffleLapkin:defer_on_drop, r=Nilstrieb
Browse files Browse the repository at this point in the history
Don't leak the function that is called on drop

It probably wasn't causing problems anyway, but still, a `// this leaks, please don't pass anything that owns memory` is not sustainable.

I could implement a version which does not require `Option`, but it would require `unsafe`, at which point it's probably not worth it.
  • Loading branch information
matthiaskrgr authored May 25, 2023
2 parents 725cadb + e2b9530 commit a9743e1
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 15 deletions.
22 changes: 14 additions & 8 deletions compiler/rustc_data_structures/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,21 +102,27 @@ pub mod unord;
pub use ena::undo_log;
pub use ena::unify;

pub struct OnDrop<F: Fn()>(pub F);
/// Returns a structure that calls `f` when dropped.
pub fn defer<F: FnOnce()>(f: F) -> OnDrop<F> {
OnDrop(Some(f))
}

pub struct OnDrop<F: FnOnce()>(Option<F>);

impl<F: Fn()> OnDrop<F> {
/// Forgets the function which prevents it from running.
/// Ensure that the function owns no memory, otherwise it will be leaked.
impl<F: FnOnce()> OnDrop<F> {
/// Disables on-drop call.
#[inline]
pub fn disable(self) {
std::mem::forget(self);
pub fn disable(mut self) {
self.0.take();
}
}

impl<F: Fn()> Drop for OnDrop<F> {
impl<F: FnOnce()> Drop for OnDrop<F> {
#[inline]
fn drop(&mut self) {
(self.0)();
if let Some(f) = self.0.take() {
f();
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_data_structures/src/owned_slice/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use std::{
};

use crate::{
defer,
owned_slice::{slice_owned, try_slice_owned, OwnedSlice},
OnDrop,
};

#[test]
Expand Down Expand Up @@ -66,7 +66,7 @@ fn boxed() {
fn drop_drops() {
let flag = Arc::new(AtomicBool::new(false));
let flag_prime = Arc::clone(&flag);
let d = OnDrop(move || flag_prime.store(true, atomic::Ordering::Relaxed));
let d = defer(move || flag_prime.store(true, atomic::Ordering::Relaxed));

let slice = slice_owned(d, |_| &[]);

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_interface/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ use crate::util;
use rustc_ast::token;
use rustc_ast::{self as ast, LitKind, MetaItemKind};
use rustc_codegen_ssa::traits::CodegenBackend;
use rustc_data_structures::defer;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::sync::Lrc;
use rustc_data_structures::OnDrop;
use rustc_errors::registry::Registry;
use rustc_errors::{ErrorGuaranteed, Handler};
use rustc_lint::LintStore;
Expand Down Expand Up @@ -325,7 +325,7 @@ pub fn run_compiler<R: Send>(config: Config, f: impl FnOnce(&Compiler) -> R + Se

rustc_span::set_source_map(compiler.sess.parse_sess.clone_source_map(), move || {
let r = {
let _sess_abort_error = OnDrop(|| {
let _sess_abort_error = defer(|| {
compiler.sess.finish_diagnostics(registry);
});

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/context/tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ where
{
TLV.with(|tlv| {
let old = tlv.replace(erase(context));
let _reset = rustc_data_structures::OnDrop(move || tlv.set(old));
let _reset = rustc_data_structures::defer(move || tlv.set(old));
f()
})
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_query_system/src/query/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use {
rustc_data_structures::fx::FxHashSet,
rustc_data_structures::sync::Lock,
rustc_data_structures::sync::Lrc,
rustc_data_structures::{jobserver, OnDrop},
rustc_data_structures::{defer, jobserver},
rustc_span::DUMMY_SP,
std::iter,
std::process,
Expand Down Expand Up @@ -530,7 +530,7 @@ fn remove_cycle<D: DepKind>(
/// all active queries for cycles before finally resuming all the waiters at once.
#[cfg(parallel_compiler)]
pub fn deadlock<D: DepKind>(query_map: QueryMap<D>, registry: &rayon_core::Registry) {
let on_panic = OnDrop(|| {
let on_panic = defer(|| {
eprintln!("deadlock handler panicked, aborting process");
process::abort();
});
Expand Down

0 comments on commit a9743e1

Please sign in to comment.