Create inotify fd with close-on-exec

[pattyshack]

resolves #155

(originally from #174)

[nathany]

Thanks :-)

[nathany]

This should be unix.IN_CLOEXEC since unix is a more up-to-date version of the syscall package.

[nathany]

Waiting on feedback/testing for ARM and PPC64 before merging.

[nathany]

Can you provide steps to manually test that file descriptors are no longer leaking with this change?

[pattyshack]

$ cat test.go
package main

import (
    "fmt"
    "log"
    "os"
    "os/exec"
    "time"

    "github.com/fsnotify/fsnotify"
)

func main() {
    if len(os.Args) == 1 { // parent
        watcher, err := fsnotify.NewWatcher()
        if err != nil {
            log.Fatal(err)
        }
        defer watcher.Close()

        child := exec.Command(os.Args[0], "child")
        err = child.Start()
        if err != nil {
            log.Fatal(err)
        }

        fmt.Println("Parent", os.Getpid(), "child", child.Process.Pid)
    }

    fmt.Println(os.Args, "going into sleep")
    for {
        time.Sleep(time.Second)
    }
}

$ ./test 
Parent 25174 child 25179
[./test] going into sleep

$ lsof -p 25174
COMMAND   PID    USER   FD      TYPE DEVICE SIZE/OFF     NODE NAME
test    25174 patrick  cwd       DIR  252,1     4096 36051491 /home/patrick/git/go/src/github.com/pattyshack/fsnotify/test
test    25174 patrick  rtd       DIR  252,1     4096        2 /
test    25174 patrick  txt       REG  252,1  2203205 12591190 /home/patrick/git/go/src/github.com/pattyshack/fsnotify/test/test
test    25174 patrick    0u      CHR  136,6      0t0        9 /dev/pts/6
test    25174 patrick    1u      CHR  136,6      0t0        9 /dev/pts/6
test    25174 patrick    2u      CHR  136,6      0t0        9 /dev/pts/6
test    25174 patrick    3r  a_inode   0,11        0     8012 inotify
test    25174 patrick    4u  a_inode   0,11        0     8012 [eventpoll]
test    25174 patrick    5r     FIFO   0,10      0t0   546148 pipe
test    25174 patrick    6w     FIFO   0,10      0t0   546148 pipe
$ lsof -p 25179
COMMAND   PID    USER   FD      TYPE DEVICE SIZE/OFF     NODE NAME
test    25179 patrick  cwd       DIR  252,1     4096 36051491 /home/patrick/git/go/src/github.com/pattyshack/fsnotify/test
test    25179 patrick  rtd       DIR  252,1     4096        2 /
test    25179 patrick  txt       REG  252,1  2203205 12591190 /home/patrick/git/go/src/github.com/pattyshack/fsnotify/test/test
test    25179 patrick    0r      CHR    1,3      0t0        6 /dev/null
test    25179 patrick    1w      CHR    1,3      0t0        6 /dev/null
test    25179 patrick    2w      CHR    1,3      0t0        6 /dev/null
test    25179 patrick    4u  a_inode   0,11        0     8012 [eventpoll]
test    25179 patrick    5r     FIFO   0,10      0t0   546148 pipe
test    25179 patrick    6w     FIFO   0,10      0t0   546148 pipe

Notice the child process does not have an inotify fd

[clnperez]

I also tested this on ppc64le and it looks like both parent and child have an inotify fd:

 ~> lsof -p 2338
COMMAND    PID    USER   FD      TYPE DEVICE SIZE/OFF    NODE NAME
fsnotify_ 2338 christy  cwd       DIR  253,2     4096  261642 /home/christy
fsnotify_ 2338 christy  rtd       DIR  253,2     4096       2 /
fsnotify_ 2338 christy  txt       REG  253,2  2351144 2373750 /tmp/go-build372706786/command-line-arguments/_obj/exe/fsnotify_inotify
fsnotify_ 2338 christy    0u      CHR  136,0      0t0       3 /dev/pts/0
fsnotify_ 2338 christy    1u      CHR  136,0      0t0       3 /dev/pts/0
fsnotify_ 2338 christy    2u      CHR  136,0      0t0       3 /dev/pts/0
fsnotify_ 2338 christy    3r  a_inode   0,11        0    6637 inotify
fsnotify_ 2338 christy    4u  a_inode   0,11        0    6637 [eventpoll]
fsnotify_ 2338 christy    5r     FIFO   0,10      0t0  432376 pipe
fsnotify_ 2338 christy    6w     FIFO   0,10      0t0  432376 pipe
 ~> lsof -p 2343
COMMAND    PID    USER   FD      TYPE DEVICE SIZE/OFF    NODE NAME
fsnotify_ 2343 christy  cwd       DIR  253,2     4096  261642 /home/christy
fsnotify_ 2343 christy  rtd       DIR  253,2     4096       2 /
fsnotify_ 2343 christy  txt       REG  253,2  2351144 2373750 /tmp/go-build372706786/command-line-arguments/_obj/exe/fsnotify_inotify
fsnotify_ 2343 christy    0r      CHR    1,3      0t0       6 /dev/null
fsnotify_ 2343 christy    1w      CHR    1,3      0t0       6 /dev/null
fsnotify_ 2343 christy    2w      CHR    1,3      0t0       6 /dev/null
fsnotify_ 2343 christy    3r  a_inode   0,11        0    6637 inotify
fsnotify_ 2343 christy    4u  a_inode   0,11        0    6637 [eventpoll]
fsnotify_ 2343 christy    5r     FIFO   0,10      0t0  432376 pipe
fsnotify_ 2343 christy    6w     FIFO   0,10      0t0  432376 pipe

[nathany]

@clnperez That's odd. I wonder if the IN_CLOEXEC flag isn't getting passed in properly or what could be causing the discrepancy on ppc.

[nathany]

I don't know why this wouldn't work. https://github.com/golang/sys/blob/master/unix/zsyscall_linux_ppc64le.go#L629

IN_CLOEXEC is defined with the same value as other platforms.
https://github.com/golang/sys/blob/master/unix/zerrors_linux_ppc64le.go#L510

So I wonder where the problem is.

[clnperez]

Yah, I don't know either. I had to add the syscalls for Pipe2 and InotifyInit, but InotifyInit1 is already there, so, hmm. I hope it just isn't something wonky on my system.

[clnperez]

Oh man. I forgot to rebuild fsnotify with this patch after I got my sys updated (since it was an older copy without the other syscalls added for ppc64le). Kiiiind of important. Sorry about the noise. It works fine on ppc64le. :)

[pattyshack]

ping (sounds like ppc64 is ok. is there any other blocker?)

[pattyshack]

thx