Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inotify IN_MODIFY not fired by ftruncate/fsync #2978

Closed
therealkenc opened this issue Feb 25, 2018 · 4 comments
Closed

inotify IN_MODIFY not fired by ftruncate/fsync #2978

therealkenc opened this issue Feb 25, 2018 · 4 comments
Assignees

Comments

@therealkenc
Copy link
Collaborator

therealkenc commented Feb 25, 2018

Version 17101. This causes journalctl -f to hang when trying to stream updates tail -f style.

Repro:

$ g++ inotify-test.cpp -o inotify-test
$ ./inotify-test
calling ppoll....
writing to foo after wakeup from nap
<hangs here>

On Real Linux you'll get a "Whee! hello world" and clean exit.

// inotify-test.cpp
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/inotify.h>
#include <sys/mman.h>
#include <unistd.h>
#include <fcntl.h>
#include <poll.h>
#include <stdlib.h>
#include <string.h>
#include <stdio.h>

int main(int argc, const char *argv[]) 
{
    const int BUF_SZ = 8192*1024;
    const char msg[] = "hello world\n\0";

    unlink("./foo");
    int fd = open("./foo", O_RDWR|O_CREAT, 0644);
    ftruncate(fd, BUF_SZ);
    char *buf = (char*)mmap(NULL, BUF_SZ, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);

    int pid = fork();
    if (pid == 0) {
        sleep(1);
        printf("writing to foo after wakeup from nap\n");
        strncpy(buf, msg, sizeof(msg));
        ftruncate(fd, BUF_SZ);
        fsync(fd);
        _exit(0);
    }

    int infd = inotify_init1(IN_NONBLOCK|IN_CLOEXEC);
    int flags = IN_MODIFY|IN_ATTRIB|IN_MOVED_TO|IN_CREATE|IN_DELETE|IN_ONLYDIR;
    int wd = inotify_add_watch(infd, ".", flags);
    if (wd < 0) {
        perror("inotify_add_watch()");
        return 1;
    }

    struct pollfd pollfd = {
        .fd = infd,
        .events = POLLIN
    };

    printf("calling ppoll....\n");
    int r = ppoll(&pollfd, 1, NULL, NULL);
    if (r < 0) {
        perror("ppol()");
        return 1;
    }

    printf("Whee! %s\n", buf);
    return 0;
}

strace:

[...usual startup stuff]
11790 unlink("./foo")                   = 0
11790 open("./foo", O_RDWR|O_CREAT, 0644) = 3
11790 ftruncate(3, 8388608)             = 0
11790 mmap(NULL, 8388608, PROT_READ|PROT_WRITE, MAP_SHARED, 3, 0) = 0x7f273e630000
11790 clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7f273f4409d0) = 11793
11793 nanosleep({1, 0},  <unfinished ...>
11790 inotify_init1(O_NONBLOCK|O_CLOEXEC) = 4
11790 inotify_add_watch(4, ".", IN_MODIFY|IN_ATTRIB|IN_MOVED_TO|IN_CREATE|IN_DELETE|IN_ONLYDIR) = 1
11790 fstat(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(136, 4), ...}) = 0
11790 brk(NULL)                         = 0x17b8000
11790 brk(0x17d9000)                    = 0x17d9000
11790 write(1, "calling ppoll....\n", 18) = 18
11790 ppoll([{fd=4, events=POLLIN}], 1, NULL, NULL, 8 <unfinished ...>
11793 <... nanosleep resumed> 0x7fffcfb1f330) = 0
11793 fstat(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(136, 4), ...}) = 0
11793 brk(NULL)                         = 0x17b8000
11793 brk(0x17d9000)                    = 0x17d9000
11793 write(1, "writing to foo after wakeup from"..., 37) = 37
11793 ftruncate(3, 8388608)             = 0
11793 fsync(3)                          = 0
11793 exit_group(0)                     = ?
11793 +++ exited with 0 +++
[...thread 11790 hangs here until you break with SIGINT]
@therealkenc
Copy link
Collaborator Author

therealkenc commented Feb 26, 2018

The test case mirrors the pattern here. I did a couple of experiments because curious, and FWIW, on Real Linux the ftruncate() is really the only call that matters; which is why they added it (see comment in the code). The fsync() is superfluous, and Linux doesn't actually take into account whether there are dirty pages. You can comment out the fsync() and everything char *buf related in the test case and it behaves exactly the same. It is mildly curious that the mask that fires is in fact IN_MODIFY (not IN_ATTRIB), whether the file was actually modified or not.

Which is to say, the fix doesn't look like it has to be a federal case. If ftruncate() is called, whether or not the length of the file changed, fire off an IN_MODIFY unilaterally. I can't think of a case that the watching end would care anyway, as long as something fires. All we are telling the watching end is: "Hey dude, this file might have been modified; maybe you should take another look at the contents".

@benhillis
Copy link
Member

Thanks Ken, this should be an easy fix. I'll take a look.

@Brian-Perkins
Copy link

This should be fixed in build 17666.

@therealkenc
Copy link
Collaborator Author

Appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants