Pidfile single process instance mechanism

Question

I'm writing a system daemon in Rust that may be started by the user manually. Only one instance of the daemon should be running at a time so I have created a pidfile locking mechanism to prevent multiple instances. This is my first time doing any kind of unix style file locking and I would appreciate another pair of eyes to check for anything I may have missed.
Style pointers would also be great as I am new to Rust.

use nix::fcntl::{flock, FlockArg};
use nix::sys::signal::kill;
use nix::unistd::Pid;
use std::fs::File;
use std::fs::OpenOptions;
use std::io;
use std::io::{Error, ErrorKind, Read, Seek, SeekFrom, Write};
use std::os::unix::io::AsRawFd;
use std::path;
use std::process;
fn write_pid(mut file: &File) -> Result<(), io::Error> {
 let id = process::id().to_string() + "\n";
 file.write_all(id.as_bytes())?;
 Ok(())
}
fn lock_pidfile(file: &File, pidfile: &path::Path) -> Result<(), io::Error> {
 flock(file.as_raw_fd(), FlockArg::LockExclusiveNonblock).map_err(|_| {
 Error::new(
 ErrorKind::Other,
 format!("Failed to lock pidfile: {}", pidfile.display()),
 )
 })
}
pub fn exclusive(pidfile: &path::Path) -> Result<bool, io::Error> {
 let pf = OpenOptions::new()
 .write(true)
 .create_new(true)
 .open(pidfile);
 match pf {
 Ok(file) => {
 lock_pidfile(&file, pidfile)?;
 write_pid(&file)?;
 Ok(true)
 }
 Err(err) => {
 match err.kind() {
 ErrorKind::AlreadyExists => {
 let mut file = OpenOptions::new().read(true).write(true).open(pidfile)?;
 lock_pidfile(&file, pidfile)?;
 let mut id_str = String::new();
 file.read_to_string(&mut id_str)?;
 let id: u32 = id_str.trim().parse().map_err(|_| {
 Error::new(
 ErrorKind::Other,
 format!("Failed to parse pidfile: {}", pidfile.display()),
 )
 })?;
 // Kill None just checks if process exists.
 // Same as kill(pid, 0); in C
 if kill(Pid::from_raw(id as i32), None).is_ok() {
 Ok(false)
 } else {
 file.seek(SeekFrom::Start(0))?;
 write_pid(&file)?;
 Ok(true)
 }
 }
 _ => Err(err),
 }
 }
 }
}
```

• hellow: use std::io; use std::io::{Error, ErrorKind, Read, Seek, SeekFrom, Write}; can be rewritten as use std::io::{self, Error, ...}. Also collapse fs::File and fs::OpenOptions.
• Fyrn: @hellow Thanks, did not think to use self less use lines make it a lot cleaner

Answer 1

I don't know much about file locking, but I can guide you in Rust.

I'd suggest changing your write_pid to use writeln!. I think you can simplify it to this.

fn write_pid(mut file: &File) -> Result<(), io::Error> {
 writeln!(file, "{}", process::id())
}

I'd also prefer to reduce the nesting in exclusive, perhaps something like this:

pub fn exclusive(pidfile: &path::Path) -> Result<bool, io::Error> {
 let pf = OpenOptions::new()
 .write(true)
 .create_new(true)
 .open(pidfile);
 match pf {
 Ok(file) => {
 lock_pidfile(&file, pidfile)?;
 write_pid(&file)?;
 return Ok(true);
 }
 Err(ref e) if e.kind() == ErrorKind::AlreadyExists => {}
 Err(e) => return Err(e),
 }
 let mut file = OpenOptions::new().read(true).write(true).open(pidfile)?;
 lock_pidfile(&file, pidfile)?;
 let mut id_str = String::new();
 file.read_to_string(&mut id_str)?;
 let id: u32 = id_str.trim().parse().map_err(|_| {
 Error::new(
 ErrorKind::Other,
 format!("Failed to parse pidfile: {}", pidfile.display()),
 )
 })?;
 // Kill None just checks if process exists.
 // Same as kill(pid, 0); in C
 if kill(Pid::from_raw(id as i32), None).is_ok() {
 Ok(false)
 } else {
 file.seek(SeekFrom::Start(0))?;
 write_pid(&file)?;
 Ok(true)
 }
}

• Fyrn: I've never seen the syntax you use here: Err(e) if e.kind() == ErrorKind::AlreadyExists => {], does the part at the end mean continue on? {] <-- this part edit: ah you probably meant {} as in do nothing.
• Fyrn: I according to the compiler this also needs ref: Err(ref e) because it can't move into the pattern guard
• Fyrn: I'm going to accept your answer because it makes the code a lot cleaner. I ended up doing my own research and found one more correctness bug: I was not checking for the case where kill fails because of a lack of permissions. I also realized that pid might not be unique, but because I want the code to be cross platform I'm going to hold off implementing process start time checking, as that is quite different between linux and macOS.
• JayDepp: Yep, sorry, I didn't run the code. I meant {} for an empty block.
• kikito: @Fyrn I want to implement the exact same feature that you did here (cross-platform single instance mechanism using pidfiles). Did you end up implementing it? Is it available somewhere?