nix-rust/nix

Replacement `nix::fcntl::Flock` does not cover all use cases for deprecated `nix::fcntl::flock`

allenap opened this issue · 2 comments

#2170 deprecated flock and replaced it with a Flock struct, allowing for automatic unlock-on-drop behaviour. So far, so good, but I think there are several ways in which flock can be used that cannot be modelled with this replacement. For example, upgrading or downgrading locks.

Suppose I have a shared lock and I want to upgrade it to an exclusive lock. With flock, if there's any error, e.g. EWOULDBLOCK/EAGAIN (same thing), I typically would not lose the existing shared lock.

However, using Flock, I cannot call Flock::lock with the Flock itself, so I would have to call Flock::unlock first to return the enclosed Flockable, before then calling Flock::lock. An error in that last step would leave me with no lock.

Some ideas for how to resolve this:

  • Remove the deprecation of flock.
  • Add a Flock::relock method that allows for upgrading/downgrading locks.
  • Add an impl Flockable for Flock<T> so that one can call Flock::lock with a Flock.

For example, upgrading or downgrading locks.

Yeah, this is something not covered by the current interface.

Some ideas for how to resolve this:

  • Remove the deprecation of flock.
  • Add a Flock::relock method that allows for upgrading/downgrading locks.
  • Add an impl Flockable for Flock so that one can call Flock::lock with a Flock.

impl Flockable for Flock<T> where T: Flockable is the most Rusty one, though Flock::unlock() would return a Ok(Flock<File>) o n success, which does not seem to be good.

So I think we should do it with the second approach, something like:

impl<T: Flockable> Flock<T> {
    /// Do another lock operation, can be used to upgrade locks.
    pub fn relock(&self, arg: FlockArg) -> Result<()> {
        let fd = self.0.as_raw_fd();
         let flags = match args {
            FlockArg::LockShared => libc::LOCK_SH,
            FlockArg::LockExclusive => libc::LOCK_EX,
            FlockArg::LockSharedNonblock => libc::LOCK_SH | libc::LOCK_NB,
            FlockArg::LockExclusiveNonblock => libc::LOCK_EX | libc::LOCK_NB,
            #[allow(deprecated)]
            FlockArg::Unlock | FlockArg::UnlockNonblock => return Err((t, Errno::EINVAL)),
        };
        Errno::result(unsafe { libc::flock(t.as_raw_fd(), flags) }).map(drop)
    }
}

but I think there are several ways in which flock can be used that cannot be modelled with this replacement.

Would you like to show me other cases that are not handled by the current impl so that we can probably also give them a fix.

Would you like to show me other cases that are not handled by the current impl so that we can probably also give them a fix.

Sorry, I suspect I was careless with my words. Right now I cannot think of another case that isn't handled.