kqueue.go: Remove timeout from reading kevents.

[paulquerna-okta]

What does this pull request do?

Instead of having a timeout that would trigger the readEvents to check it's
done channel, run a seaprate go-routine that watches w.done, and directly
closes the KQueue FD.

This change allows a go-process to fully sleep and go into CPU idle mode,
instead of waking up every 100ms on macOS.

Fixes #237

[gdey]

LGTM

[paulquerna-okta]

Closing as this was merged to master

wasn't actually merged to master -- issue is still there.

[wweir]

Expect a new release. This problem has caused me a big headache.

[paulquerna-okta]

Is it possible to merge this? Just rebased it against master, and we've been running with this patch on our software for about 8 months now.

[nathany]

Thanks @paulquerna-okta @wweir and @gdey.

[paulquerna-okta]

Rebased against current master -- just a note we've been running this patch for 18 months to reduce CPU usage on our MacOS app. Without it the app would consistently show up Activity Monitor's Energy tab.

[dmt0]

Any chance this could be merged in?

[arp242]

On FreeBSD and NetBSD this blocks forever on:

n, err := unix.Kevent(kq, nil, events, timeout)

Called from the main event loop:

kevents, err := read(w.kq(), eventBuffer, nil)

kqueue(2) on FreeBSD says:

If timeout is a non-NULL pointer, it specifies a maximum interval to wait for an event, which will be interpreted as a struct timespec. If timeout is a NULL pointer, kevent() waits indefinitely.

kqueue(2) on OpenBSD and macOS have the same language; I added some debug printf statements, and got:

OpenBSD:

read ...  readDone <nil> 1
read ...  readDone <nil> 1
read ...  readDone <nil> 2
read ...  readDone <nil> 1
read ...  readDone <nil> 1
read ...  readDone <nil> 1
read ...closeKq ....  readDone bad file descriptor -1
closeKqDone: <nil>

FreeBSD:

read ...  readDone <nil> 1
read ...  readDone <nil> 1
read ...  readDone <nil> 2
read ...  readDone <nil> 1
read ...  readDone <nil> 1
read ...  readDone <nil> 1
read ...closeKq ....closeKqDone: <nil>
    helpers_test.go:278: event stream was not closed after 1 second

So the difference is that on OpenBSD Kevent() will return if the file descriptor is closed, whereas on FreeBSD it won't. I assume it's the same on macOS, but I didn't try that yet.

I can't really find this behaviour documented anywhere, and seems to be something of an implementation detail that may change on macOS and OpenBSD in the future too.

---

Some other (minor) issues:

• It seems to call unix.Close() on _kq twice? once in closeKq() and once at the end of readEvents(). You added a unix.EBADF error check for this, but that's only needed because it's already closed and set to -1 in closeKq().

• Also, the timeout parameters is now always nil, and durationToTimespec() is never used (but those are minor issues, compared to it not working at all).

---

And my printf debugging, in case it's useful to anyone else:

diff --git kqueue.go kqueue.go
index 1d19378..a3aa768 100644
--- kqueue.go
+++ kqueue.go
@@ -304,7 +304,9 @@ func (w *Watcher) kq() int {
func (w *Watcher) closeKq() {
        <-w.done
        w.mu.Lock()
-       unix.Close(w._kq)
+       fmt.Print("closeKq ....")
+       err := unix.Close(w._kq)
+       fmt.Println("closeKqDone:", err)
        w._kq = -1
        w.mu.Unlock()
}
@@ -560,7 +562,9 @@ func register(kq int, fds []int, flags int, fflags uint32) error {
// read retrieves pending events, or waits until an event occurs.
// A timeout of nil blocks indefinitely, while 0 polls the queue.
func read(kq int, events []unix.Kevent_t, timeout *unix.Timespec) ([]unix.Kevent_t, error) {
+       fmt.Print("read ...")
        n, err := unix.Kevent(kq, nil, events, timeout)
+       fmt.Printf("  readDone %v %d\n", err, n)
        if err != nil {
                return nil, err
        }

[arp242]

#124 seems to fix the same problem, and doesn't seem to have any of the above issues in a quick check. Updated branch here, but needs some more work as it's a pretty old branch: https://github.com/fsnotify/fsnotify/tree/kqueue-no-timeout

[arp242]

I created #480, which should solve this issue better.