"unused" variables that need to not be prematurely dropped

I have some code in a function called create_pid_file that opens a file using flock(), and returns the file, or an error if it fails. The purpose of this is to ensure that only one copy of the program is running at a time, and to do this the caller must keep the file returned in scope (i.e. open) until the program exits.

Previously, since this file isn't actually used for any other purpose, the caller did:

let _ = create_pid_file()?;

but this recently (rust 1.25 maybe?) stopped working. The file is closed immediately, which created a bug. I changed the code to:

let _pidfile = create_pid_file()?;

and this works again, but... _pidfile is still not being used. Does Rust make any guarantees that it won't drop it before it goes out of scope? What can I do here to get the desired behavior, going forward? I'm worried about the optimizer getting even smarter :smiley:

1 Like

I'm surprised your original code worked at some point. This code prints the same in Rust 1.0.0 and 1.25.0 in debug and release mode:

struct DropPrint;

impl Drop for DropPrint {
	fn drop(&mut self) {
		println!("in drop");
	}
}

fn a() {
	let _ = DropPrint;
	println!("after let");
}

fn b() {
	let _v = DropPrint;
	println!("after let");
}

fn main() {
	println!("before a");
	a();
	println!("before b");
	b();
}

output:

before a
in drop
after let
before b
after let
in drop

This behavior is as expected, only if you create a binding will the value live until the end of the scope. The pattern _ doesn't create a binding.

If you want to be super-duper sure when your value gets dropped, you can be explicit and add drop(pidfile) at the end of your function.

3 Likes

Thanks! I guess I must have mis-remembered exactly the code I originally tested as working.

Two safer alternatives:


Invert the API

Basically, don't return the PID file, so that it is impossible to drop it.

with_pid_file(|| {
    // code that executes while the lockfile is held
})

Stop "not using" the return value!

Operations that require a lock to be held should take that lockfile (or a borrow of it) as an argument, as proof that the lockfile is held.

let lock = create_pid_file()?;

do_sensitive_operation(&lock)?;

Equivalently, you can have create_pid_file return a wrapper type around the lockfile, and implement sensitive operations as methods on that wrapper type.

6 Likes

let _ = is a dedicated immediate-drop construct, so it's not merely optimizer being clever.

Rust has a guaranteed order of drops, so let _pidfile = should be safe. If you want to make it 100% explicit, you can add drop(pidfile) at the end of the function.

2 Likes

@ExpHO "make invalid states unrepresentable", I like it!

1 Like

To elaborate on the mental model here, let _ never binds in the first place. This means that the RHS is a temporary, and so is gonna be dropped right away. But let _pidfile does actually bind, to _pidfile, and so it will drop when _piidfile goes out of scope.

At the risk of beating a dead horse, and to add to @steveklabnik's mental model (or maybe make it more explicit):

let _ = create_pid_file()?;

is the same thing as

create_pid_file()?;

There's no binding there at all, and the result is dropped on the spot. The let _ is merely a way to tell the compiler you're explicitly ignoring the return value, and thus avoid the warning. Otherwise, it's the same thing and perhaps seeing it without a binding makes it clearer that nothing "holds" the pid file guard.

Will it still work with NLLs?

Will it still work with NLLs?

Yes. NLL does not change when things are dropped.

1 Like