os/inotify: inotify for Windows (issue4188047)

[hect...@gmail.com]

Reviewers: rsc, brainman, leczb, golang-dev_googlegroups.com,

Message:
Hi guys,

Just a heads up about this largish code submission for inotify on
Windows. The code is ready to review, however I modified the test and
it currently fails on Linux. The failure isn't too serious in my
opinion - it fails because the Event channel won't close after the
watcher is closed. I will look into it a bit more tomorrow evening.

Description:
os/inotify: inotify for Windows

Please review this at http://codereview.appspot.com/4188047/

Affected files:
M src/pkg/Makefile
M src/pkg/os/inotify/Makefile
M src/pkg/os/inotify/inotify_linux_test.go
A src/pkg/os/inotify/inotify_windows.go
M src/pkg/syscall/syscall_windows.go
M src/pkg/syscall/zsyscall_windows_386.go
M src/pkg/syscall/ztypes_windows_386.go

[hect...@gmail.com]

To follow up on my earlier email, I believe there is a bug in the Linux
implementation where if a watcher has no watches then calling Close on
it doesn't actually cause the reader goroutine to quit. I have added a
fix for this in Patch Set 2.

http://codereview.appspot.com/4188047/

[alex.b...@gmail.com]

Thank you for doing this.

> ... I have added a fix for

> this in Patch Set 2.

Linux test fails for me:

--- FAIL: inotify.TestInotifyEvents (3.1 seconds)
expected event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x100
received event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x100 ==
IN_CREATE
expected event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x2
received event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x2 ==
IN_MODIFY
expected event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x2
received event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x2 ==
IN_MODIFY
expected event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x40
received event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x40 ==
IN_MOVED_FROM
expected event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile.new": 0x80
received event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile.new": 0x80
== IN_MOVED_TO
expected event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x800
received event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x800 ==
IN_MOVE_SELF
expected event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x400
received event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x400 ==
IN_DELETE_SELF
expected event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x8000
received event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x8000 ==
IN_IGNORED
expected event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile.new": 0x200
received event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile.new": 0x200
== IN_DELETE
expected event: "TestInotifyEvents.testdirectory": 0x400
received event: "TestInotifyEvents.testdirectory": 0x400 ==
IN_DELETE_SELF
expected event: "TestInotifyEvents.testdirectory": 0x8000
received event: "TestInotifyEvents.testdirectory": 0x8000 == IN_IGNORED
calling Close()
waiting for the event channel to become closed...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
waiting for 50 ms...
event stream was not closed after 1 second, as expected
FAIL
make: *** [test] Error 1

So does windows test:

--- FAIL: inotify.TestInotifyEvents (0.2 seconds)
expected event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x100
error received: GetQueuedCompletionPort: An unexpected network error
occurred.
FAIL

If I trace all syscall in windows, this is what I get:

SYSCALL: GetStdHandle(stdhandle=-10) (handle=3, errno=0)
SYSCALL: GetStdHandle(stdhandle=-11) (handle=7, errno=0)
SYSCALL: GetStdHandle(stdhandle=-12) (handle=24, errno=0)
SYSCALL: GetCommandLine() (cmd=0x209f4)
SYSCALL: CommandLineToArgv(cmd=0x209f4, argc=0x109c0178) (argv=0x73dd8,
errno=0)
SYSCALL: LocalFree(hmem=474584) (handle=0, errno=0)
SYSCALL: GetSystemTimeAsFileTime(time=0x109c02d0) ()
SYSCALL: CreateIoCompletionPort(filehandle=-1, cphandle=0, key=0,
threadcnt=0) (handle=676, errno=0)
SYSCALL: DeleteFile(path=0x109c4900) (errno=5)
SYSCALL: RemoveDirectory(path=0x109c4f00) (errno=145)
SYSCALL: FindFirstFile(name=0x109c4ec0, data=0x109c8c80) (handle=481640,
errno=0)
SYSCALL: FindClose(handle=481640) (errno=0)
SYSCALL: FindFirstFile(name=0x109c5e10, data=0x109c8a00) (handle=481640,
errno=0)
SYSCALL: FindNextFile(handle=481640, data=0x109c8a00) (errno=0)
SYSCALL: FindNextFile(handle=481640, data=0x109c8a00) (errno=0)
SYSCALL: FindNextFile(handle=481640, data=0x109c8a00) (errno=18)
SYSCALL: DeleteFile(path=0x109c7380) (errno=0)
SYSCALL: FindNextFile(handle=481640, data=0x109c8a00) (errno=18)
SYSCALL: FindClose(handle=481640) (errno=0)
SYSCALL: DeleteFile(path=0x109c4e00) (errno=5)
SYSCALL: RemoveDirectory(path=0x109c4dc0) (errno=0)
SYSCALL: CreateDirectory(path=0x109c4d80, sa=0x0) (errno=0)
SYSCALL: PostQueuedCompletionStatus(port=676, count=0, key=0,
overlapped=0x0) (errno=0)
SYSCALL: GetQueuedCompletionStatus(cphandle=676, qty=0x109c0260,
key=0x109c0258, overlapped=0x109c0240, timeout=4294967295) (errno=0)
SYSCALL: FindFirstFile(name=0x10a06600, data=0x109ef000) (handle=481640,
errno=0)
SYSCALL: FindClose(handle=481640) (errno=0)
SYSCALL: CreateFile(name=0x10a065c0, access=1, mode=7, sa=0x0,
createmode=3, attrs=1107296256, templatefile=0) (handle=644, errno=0)
SYSCALL: GetFileInformationByHandle(handle=644, data=0x10a06580)
(errno=0)
SYSCALL: CreateIoCompletionPort(filehandle=644, cphandle=676, key=0,
threadcnt=0) (handle=676, errno=0)
SYSCALL: CancelIo(s=644) (errno=0)
SYSCALL: ReadDirectoryChanges(handle=644, buf=0x10a1b034, buflen=4096,
watchSubTree=false, mask=51, retlen=0x0, overlapped=0x10a1b000,
completionRoutine=0) (errno=0)
SYSCALL: FindFirstFile(name=0x109c7580, data=0x109c8780) (handle=-1,
errno=3)
SYSCALL: CreateFile(name=0x109c7600, access=1073741824, mode=3, sa=0x0,
createmode=2, attrs=128, templatefile=0) (handle=624, errno=0)
SYSCALL: GetQueuedCompletionStatus(cphandle=676, qty=0x109c0260,
key=0x109c0258, overlapped=0x109c0240, timeout=4294967295) (errno=59)
SYSCALL: PostQueuedCompletionStatus(port=676, count=0, key=0,
overlapped=0x0) (errno=0)
SYSCALL: GetQueuedCompletionStatus(cphandle=676, qty=0x109c0260,
key=0x109c0258, overlapped=0x109c0240, timeout=4294967295) (errno=0)
SYSCALL: FormatMessage(flags=12800, msgsrc=0, msgid=59, langid=0,
buf=[300/300]0x109c8500, args=0x0) (n=39, errno=0)
SYSCALL: FindFirstFile(name=0x10a19180, data=0x109ef500) (handle=481640,
errno=0)
SYSCALL: GetSystemTimeAsFileTime(time=0x109c02f8) ()
--- FAIL: inotify.TestInotifyEvents (0.9 seconds)
expected event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testfile": 0x100
error received: GetQueuedCompletionPort: An unexpected network error
occurred.
SYSCALL: FindClose(handle=481640) (errno=0)
SYSCALL: GetSystemTimeAsFileTime(time=0x109c0548) ()
SYSCALL: CreateFile(name=0x10a064c0, access=1, mode=7, sa=0x0,
createmode=3, attrs=1107296256, templatefile=0) (handle=616, errno=0)
SYSCALL: CreateIoCompletionPort(filehandle=-1, cphandle=0, key=0,
threadcnt=0) (handle=608, errno=0)
SYSCALL: GetFileInformationByHandle(handle=616, data=0x10a06480)
(errno=0)
SYSCALL: PostQueuedCompletionStatus(port=608, count=0, key=0,
overlapped=0x0) (errno=0)
SYSCALL: GetQueuedCompletionStatus(cphandle=608, qty=0x109c0438,
key=0x109c0440, overlapped=0x109c0448, timeout=4294967295) (errno=0)
SYSCALL: CloseHandle(handle=616) (errno=0)
SYSCALL: GetSystemTimeAsFileTime(time=0x109c0570) ()
SYSCALL: CloseHandle(handle=608) (errno=0)
SYSCALL: CancelIo(s=644) (errno=0)
SYSCALL: ReadDirectoryChanges(handle=644, buf=0x10a1b034, buflen=4096,
watchSubTree=false, mask=51, retlen=0x0, overlapped=0x10a1b000,
completionRoutine=0) (errno=0)
SYSCALL: sleep(msec=50) ()
SYSCALL: GetSystemTimeAsFileTime(time=0x109c0580) ()
SYSCALL: GetQueuedCompletionStatus(cphandle=676, qty=0x109c0260,
key=0x109c0258, overlapped=0x109c0240, timeout=4294967295) (errno=59)
SYSCALL: WriteFile(handle=624, buf=[12/13]0x109ee2c0, done=0x109c0458,
overlapped=0x0) (errno=0)
SYSCALL: GetSystemTimeAsFileTime(time=0x109c0598) ()
FAIL
SYSCALL: CloseHandle(

Alex

http://codereview.appspot.com/4188047/diff/4015/src/pkg/Makefile
File src/pkg/Makefile (right):

http://codereview.appspot.com/4188047/diff/4015/src/pkg/Makefile#newcode195
src/pkg/Makefile:195: DIRS+=os/inotify
Your action doesn't match the comment. Please just put

ifeq ($(GOOS),windows)
DIRS+=os/inotify
endif

right next to similar linux ifeq for inotify.

http://codereview.appspot.com/4188047/diff/4015/src/pkg/os/inotify/inotify_linux_test.go
File src/pkg/os/inotify/inotify_linux_test.go (left):

http://codereview.appspot.com/4188047/diff/4015/src/pkg/os/inotify/inotify_linux_test.go#oldcode6
src/pkg/os/inotify/inotify_linux_test.go:6:
Should, probably, rename this file to inotify_test.go.

http://codereview.appspot.com/4188047/

[Hector Chu]

Thanks for reviewing and testing this Alex.  I'm surprised that my fix for Linux isn't working for you - I wonder if Balazs can chip in on this?  As for Windows, it sounds like the test is being run from a Linux share.  Unfortunately this is not a scenario that is guaranteed to work.  Can you try running the test from a local hard drive please?

Thanks,
Hector

On 15 Feb 2011 00:42, <alex.b...@gmail.com> wrote:

[peterGo]

Hector,

For ./all.bash on Windows 7, I get:

cd os/inotify && make test
make[1]: Entering directory `/c/inotify/src/pkg/os/inotify'
gotest
make[2]: Entering directory `/c/inotify/src/pkg/os/inotify'
rm -f _test/os/inotify.a _gotest_.8
make[2]: Leaving directory `/c/inotify/src/pkg/os/inotify'
make[2]: Entering directory `/c/inotify/src/pkg/os/inotify'
8g -o _gotest_.8 inotify_windows.go inotify_linux_test.go
rm -f _test/os/inotify.a
gopack grc _test/os/inotify.a _gotest_.8
make[2]: Leaving directory `/c/inotify/src/pkg/os/inotify'
PASS
make[1]: Leaving directory `/c/inotify/src/pkg/os/inotify'

Peter

> On 15 Feb 2011 00:42, <alex.brain...@gmail.com> wrote:

[Hector Chu]

Hi Peter,

Cool, thanks for the report.

Thanks,
Hector

[peterGo]

Hector,

For ./all.bash on Linux (Ubuntu 10.04, 64-bit), I get:

cd os/inotify && make test

make[1]: Entering directory `/home/peter/inotify/src/pkg/os/inotify'
gotest
make[2]: Entering directory `/home/peter/inotify/src/pkg/os/inotify'
rm -f _test/os/inotify.a _gotest_.6
make[2]: Leaving directory `/home/peter/inotify/src/pkg/os/inotify'
make[2]: Entering directory `/home/peter/inotify/src/pkg/os/inotify'
6g -o _gotest_.6 inotify_linux.go inotify_linux_test.go
rm -f _test/os/inotify.a
gopack grc _test/os/inotify.a _gotest_.6
make[2]: Leaving directory `/home/peter/inotify/src/pkg/os/inotify'
PASS
make[1]: Leaving directory `/home/peter/inotify/src/pkg/os/inotify'

Peter

On Feb 15, 2:53 am, Hector Chu <hector...@gmail.com> wrote:

> On 15 Feb 2011 00:42, <alex.brain...@gmail.com> wrote:

[go.pe...@gmail.com]

Alex,

When I run ./all.bash for Windows XP, Windows 7, Ubuntu 10.10, 32-bit,
and Ubuntu 10.04, 64-bit, all tests, including inotify test, pass.

What are doing for your tests?

Peter

[alex.b...@gmail.com]

On 2011/02/15 07:53:42, hector wrote:
> ... I'm surprised that my fix for

> Linux isn't working for you - I wonder if Balazs can chip in on this?

...

FYI, my uname -a prints:

Linux sos 2.6.24-gentoo-r8 #10 SMP Thu Jan 22 16:09:08 EST 2009 i686
Intel(R) Xeon(TM) CPU 1.80GHz GenuineIntel GNU/Linux

> ... As

> for Windows, it sounds like the test is being run from a Linux share.

I did do that.

> Unfortunately this is not a scenario that is guaranteed to work. Can
you
> try running the test from a local hard drive please?

Tried that. Sometimes it PASSes, sometimes it fails with this message:

C:\TMP\t2>g:\src\pkg\os\inotify\8.out.exe -v
=== RUN inotify.TestInotifyEvents

--- FAIL: inotify.TestInotifyEvents (0.2 seconds)
expected event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testf
ile": 0x100

received event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testf
ile": 0x100 == IN_CREATE

expected event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testf
ile": 0x2

expected event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.testf
ile": 0x2, received event:
"TestInotifyEvents.testdirectory/TestInotifyEvents.te
stfile": 0x100 == IN_CREATE
=== RUN inotify.TestInotifyClose
--- PASS: inotify.TestInotifyClose (0.1 seconds)
FAIL

C:\TMP\t2>

Windows 2000 here.

Alex

http://codereview.appspot.com/4188047/

[alex.b...@gmail.com]

On 2011/02/16 00:56:35, peterGo wrote:

> What are doing for your tests?

I build test on linux:

cd $GOROOT/src/pkg/os/inotify
export GOOS=windows
export GOARCH=386
make clean install test

And then run built executable (8.out.exe) on my Windows PC.

Alex

http://codereview.appspot.com/4188047/

[go.pe...@gmail.com]

Alex,

Your Windows PC is running Microsoft Windows 2000, an obsolete operating
system whose extended support ended on August 13, 2010.

I have tested 8.out.exe on Windows 2000, XP, and 7. It only fails on
Windows 2000. There is no reason to support an obsolete and unsupported
operating system.

Peter

On 2011/02/16 01:03:39, brainman wrote:
> On 2011/02/16 00:56:35, peterGo wrote:
> >
> > What are doing for your tests?
> >

> I build test on linux:

> cd $GOROOT/src/pkg/os/inotify
> export GOOS=windows
> export GOARCH=386
> make clean install test

> And then run built executable (8.out.exe) on my Windows PC.

> Alex

[alex.b...@gmail.com]

On 2011/02/16 01:51:43, peterGo wrote:
> ... It only fails on Windows

> 2000. There is no reason to support an obsolete and unsupported
operating
> system.

I think the onus is on us to understand why and only then we can make
such decision.

Alex

http://codereview.appspot.com/4188047/

[Benny Siegert]

On Wed, Feb 16, 2011 at 02:51, <go.pe...@gmail.com> wrote:
> Your Windows PC is running Microsoft Windows 2000, an obsolete operating
> system whose extended support ended on August 13, 2010.
>
> I have tested 8.out.exe on Windows 2000, XP, and 7. It only fails on
> Windows 2000. There is no reason to support an obsolete and unsupported
> operating system.

The fact that it has users (including Alex!) means that there is a reason.

--Benny.

[Hector Chu]

Thanks all. I think the Win2k issue is caused by a simple race in the
code, and I already have a fix in mind. As for Linux, I will try a
different approach.

Thanks,
Hector

[Russ Cox]

I'm a little uncomfortable with the idea of "inotify for Windows".
inotify is a very Linux-specific thing and shoehorning Windows
into that sets a bad precedent. I don't want to just take what
Linux does as the official interface without an explicit decision.
The intent was to get some experience with what inotify would
look like, not to set the API for all other OSes.

What other OSes have similar notification mechanisms?
Let's try to sketch what the generic os/notify API is and
make sure it can be implemented and exposes enough
on the various systems.

The code looks fine, I just want to make sure we get the
interface right.

Russ

[Hector Chu]

Ok, understood. To start things off, I looked briefly at the kind of
support that the other OSes give for file system notification.

Mac:
See how it is implemented in Factor:
http://factor-language.blogspot.com/2008/02/file-system-change-monitoring-on-mac-os.html
According to the blog post it appears to contain severe limitations.
It requires a CoreFoundation event loop to be running. Also it won't
tell you the names of the files that changed and what the changes
were, only that a change occurred within a directory. It's not a
syscall interface, but rather a Carbon API.
There appears to be an alternative called kqueue but I think it has
the same downsides as Linux dnotify.

FreeBSD:
It appears to provide a clone of inotify called fsnotify.
(Coincidentally Linux inotify itself was reimplemented on top of a new
notify framework called fsnotify in 2008). It doesn't appear to be an
official API yet as I couldn't find the man page for it.

[alex.b...@gmail.com]

I haven't used any such technology in the past, but

http://tinyurl.com/4onwbvr

paints pretty good picture for Windows (see links on the bottom). It all
comes down to ReadDirectoryChangesW that hector used in his proposal.

I can see 2 issues with it already:

- it will not work for some network "shares":

"... Samba servers will not generate notifications, probably because the
underlying operating system does not support the functionality. Network
Attached Storage (NAS) devices usually run Linux, so won't support
notifications. High-end SANs are anybody's guess..."

- there known problems with ReadDirectoryChangesW:

"...There are many posts on the internet about the ReadDirectoryChangesW
API function missing files when there is a lot of file activity...".

Also, it is quite complicated to use ReadDirectoryChangesW interface.

I'm not sure how popular os/inotify is (will be) with Go users, but
maybe it shouldn't be part of standard library. Maybe we should leave it
the way it is now, and only invest in it later once we get some user
feedback.

Alex

http://codereview.appspot.com/4188047/

[Russ Cox]

> I can see 2 issues with it already:
>
> - it will not work for some network "shares":
>
> "... Samba servers will not generate notifications, probably because the
> underlying operating system does not support the functionality. Network
> Attached Storage (NAS) devices usually run Linux, so won't support
> notifications. High-end SANs are anybody's guess..."

This is true even when Linux is the operating system:
what happens on the network typically doesn't get reported.

> - there known problems with ReadDirectoryChangesW:
>
> "...There are many posts on the internet about the ReadDirectoryChangesW
> API function missing files when there is a lot of file activity...".

It's okay if Go exposes the same bugs Windows has.

> Also, it is quite complicated to use ReadDirectoryChangesW interface.
>
> I'm not sure how popular os/inotify is (will be) with Go users, but
> maybe it shouldn't be part of standard library. Maybe we should leave it
> the way it is now, and only invest in it later once we get some user
> feedback.

I like how simple it ended up, and I know there are people who
do use it (because they wrote it). I think we can end up with
a decent API and keep it.

Will think more tomorrow.

Russ

[Benny Siegert]

On Thu, Feb 17, 2011 at 01:56, Hector Chu <hect...@gmail.com> wrote:
> Mac:
[...]

> There appears to be an alternative called kqueue but I think it has
> the same downsides as Linux dnotify.
>
> FreeBSD:
> It appears to provide a clone of inotify called fsnotify.

The BSD-based operating systems (FreeBSD and Darwin among the
architectures supported by Go) use kqueue as the notification
mechanism. See http://people.freebsd.org/~jlemon/papers/kqueue.pdf.

--Benny.

[Hector Chu]

I think a sensible interface for Go would have us only watching directories.  We probably shouldn't allow recursive directory watching.  Then we should expose a limited subset of the most useful flags in inotify.  These would probably be create, delete, modify and rename (within the directory only).  Conveniently this is the intersection of flags that Linux and Windows supports (of course we would check that these notifications are also supported under the other OSes).  We should also merge the Event and Error channels.  Is there anything else that we need to address?

Thanks,
Hector

[mattn]

Why you don't use FindFirstChangeNotification() FindCloseChangeNotification(), FindNextChangeNotification() and WaitForSingleObject in goroutine?