Page MenuHomeFreeBSD

Allow flock() on fifos
ClosedPublic

Authored by greg_unrelenting.technology on Apr 1 2020, 11:07 PM.

Details

Summary

flock currently does not work on named pipes. vop_advlock handles both flock and the weird granular F_SETLK stuff — I guess the latter does not make sense for pipes, which is why vop_advlock has been disabled on fifos since forever. But flock makes sense and some software does it on Linux.

kern_fcntl has its own (fp->f_type != DTYPE_VNODE) check on F_SETLK/F_GETLK, so I thiiiink I'm not enabling too much?? I can't think of a way to get to the vop_advlock with non-flock args through a different syscall..

Test Plan

For the happy case, build this:

#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/file.h>
#define PATH "/tmp/locktest.fifo"

int main() {
        mkfifo(PATH, 0666);
        int fd = open(PATH, O_WRONLY);
        fprintf(stderr, "open\n");
        if (flock(fd, LOCK_EX) != 0) fprintf(stderr, "flock: %d %s\n", errno, strerror(errno));
        fprintf(stderr, "lock\n");
        int cnt = 420;
        while (cnt--) { write(fd, ".", sizeof(".")); usleep(10000); }
        if (flock(fd, LOCK_UN) != 0) fprintf(stderr, "flock: %d %s\n", errno, strerror(errno));
        fprintf(stderr, "unlock\n");
        return 0;
}

launch two instances in two terminals and cat /tmp/locktest.fifo in a third one. Observe only one instance writing at a time.

for the other kinds of locking.. idk

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Sorry for late reply, I'll look into it. In general, unless there is a reason not to, we should be feature-compatible with Linux so this (and perhaps more) is on the table.

I agree with the patch. What line would like to see in "submitted by"?

In D24255#565138, @mjg wrote:

I agree with the patch. What line would like to see in "submitted by"?

Whatever you want :D Usually it's 'Greg V <greg@unrelenting.technology>' though sometimes the @ was replaced with a _ like in the phabricator generated username..

mjg requested changes to this revision.EditedJul 5 2020, 6:29 AM

I perused the code a little more before committing. The whole vs range locking thing is making me increasingly worried. That said, I think the best course of action is to add validation to fifo_advlock that the args passed match up what's de facto passable by flock. Then it can call vop_stdadvlock.

vn close will need adjusting as well:

ref= (fp->f_flag & FHASLOCK) != 0 && fp->f_type == DTYPE_VNODE;

I think the vnode check can be dropped, instead FHASLOCK can only be set on vnodes which get advlock.

This revision now requires changes to proceed.Jul 5 2020, 6:29 AM

Hi. Do you need help updating the patch? I can do the necessary changes.

In D24255#575592, @mjg wrote:

Hi. Do you need help updating the patch? I can do the necessary changes.

Yeah, please do.

This revision was not accepted when it landed; it landed in state Needs Revision.Sep 25 2021, 3:37 PM
Closed by commit rGd71e1a883c92: fifo: support flock (authored by mjg). · Explain Why
This revision was automatically updated to reflect the committed changes.