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
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; 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 R10:d71e1a883c92: fifo: support flock (authored by mjg). · Explain Why
This revision was automatically updated to reflect the committed changes.