Skip to content
This repository has been archived by the owner on Jun 8, 2022. It is now read-only.

In fsnotify_windows/readEvents(), GetQueuedCompletionStatus sometimes returns ERROR_MORE_DATA #49

Closed
jbowtie opened this issue Jun 7, 2013 · 7 comments

Comments

@jbowtie
Copy link
Contributor

jbowtie commented Jun 7, 2013

Note that this error is not defined in syscall/ztypes_windows.go for some reason, you can declare as follows:

const (
    ERROR_MORE_DATA syscall.Errno = 234
)

When GetQueuedCompletionStatus returns ERROR_MORE_DATA and lpOverlapped is not null, the buffer is full but n will still be 0 (really it should be the buffer size but that's Windows for you).

In my case it's is sufficient to just modify n and carry on, though it carries the risk of running off the end of the buffer during event processing:

case ERROR_MORE_DATA:
    if watch == nil {
        w.Error <- errors.New("ERROR_MORE_DATA with null ov")
    } else {
        n = 4096
    }

However a proper solution would be to fetch the additional data (presumably by looping while ERROR_MORE_DATA and accumulating buffers) before we start processing events.

@howeyc
Copy link
Owner

howeyc commented Jun 9, 2013

I've been reading up on the windows docs, but have no idea what this is or how to produce it. Would you be open to creating a pull request with the changes you made? That may help me see the proper solution you are proposing.

@jbowtie
Copy link
Contributor Author

jbowtie commented Jun 10, 2013

This is better documented in the context of ReadFile, where it is a relatively common. You can look at http://msdn.microsoft.com/en-us/library/windows/desktop/aa364052(v=vs.85).aspx as a decent starting place.

In summary, ERROR_MORE_DATA is returned whenever the buffer in the lpOverlapped structure is full; ideally you keep calling to get the rest of the data. You can see an example of this using ReadFile in the named pipe client example http://msdn.microsoft.com/en-us/library/windows/desktop/aa365592(v=vs.85).aspx

I don't have a reproduction recipe but it's almost definitely related to watching a directory on a network share.

I'll set up a pull request shortly.

jbowtie added a commit to jbowtie/fsnotify that referenced this issue Jun 10, 2013
See issue howeyc#49 for background and discussion
@howeyc
Copy link
Owner

howeyc commented Jun 11, 2013

I think what you've suggested may actually work, since it will keep looping to get more data after processing what it was able to read.

I understand you may not be able to readily reproduce this, but could you try an extra update I have on the "windows-moredata" branch? I want to see if it's possibly to have the system call return events that could push over the edge of the buffer.

@jbowtie
Copy link
Contributor Author

jbowtie commented Jun 13, 2013

I've just tried that branch. I had to modify one line to convert between int types:

n = uint32(len(watch.buf))

With the reduced buffer size it's easy to get a buffer overrun simply by using a sufficiently long filename, such as my test case of ZZZ-test-a-long-sample-filename-for-file-monitor-need-to-be-longer-than-128-bytes-to-ensure-overflow-packet-so-making-sure-we-wrap-to-2-lines.xml

error: short read in readEvents()

@howeyc
Copy link
Owner

howeyc commented Jun 16, 2013

When I attempt to reproduce, I am able to get the short read, but not an ERROR_MORE_DATA. As such, I have made updates to the "windows-moredata" branch to be what I think should be correct.

I refer to your good judgement whether this is satisfactory, and if so, I will merge into master.

@jbowtie
Copy link
Contributor Author

jbowtie commented Jun 16, 2013

This solution is sufficient to my use case. It would be nice if this line could be a warning as opposed to an error (also typo, should be "occurred" with two r's) but probably not important enough to worry about:

w.Error <- errors.New("ERROR_MORE_DATA has occured, assuming full buffer returned from system")

@howeyc
Copy link
Owner

howeyc commented Jun 17, 2013

Thanks for working on this.

@howeyc howeyc closed this as completed Jun 17, 2013
rsc pushed a commit to golang/exp that referenced this issue Dec 7, 2014
Handle ERROR_MORE_DATA on Windows
(howeyc/fsnotify#49)

Run tests in random temp directories
(howeyc/fsnotify#57)

Fix: RemoveWatch is not removing the path from the watch list
The issue was that files watched internally were not being removed
when the parent directory's watch was removed.
(howeyc/fsnotify#71)

Fix: Race on OS X between Close() and readEvents()
(howeyc/fsnotify#70)

Fix: deadlock on BSD
The removeWatch routine could return without releasing the lock on
w.bufmut. This change unlocks the mutex before checking for errors.
(howeyc/fsnotify#77)

Add an IsAttrib method on the FileEvent struct
(howeyc/fsnotify#79)

Fix: a few typos

Test helpers for shared setup.

LGTM=iant
R=golang-codereviews, dave, alex.brainman, gobot, bradfitz, iant
CC=bradfitz, bronze1man, cespare, denis.brandolini, golang-codereviews, henrik.edwards, jbowtie, travis.cline, webustany
https://golang.org/cl/58500043
GoogleCodeExporter pushed a commit to bsed/go-zh.exp that referenced this issue May 31, 2015
Handle ERROR_MORE_DATA on Windows
(howeyc/fsnotify#49)

Run tests in random temp directories
(howeyc/fsnotify#57)

Fix: RemoveWatch is not removing the path from the watch list
The issue was that files watched internally were not being removed
when the parent directory's watch was removed.
(howeyc/fsnotify#71)

Fix: Race on OS X between Close() and readEvents()
(howeyc/fsnotify#70)

Fix: deadlock on BSD
The removeWatch routine could return without releasing the lock on
w.bufmut. This change unlocks the mutex before checking for errors.
(howeyc/fsnotify#77)

Add an IsAttrib method on the FileEvent struct
(howeyc/fsnotify#79)

Fix: a few typos

Test helpers for shared setup.

LGTM=iant
R=golang-codereviews, dave, alex.brainman, gobot, bradfitz, iant
CC=bradfitz, bronze1man, cespare, denis.brandolini, golang-codereviews, henrik.edwards, jbowtie, travis.cline, webustany
https://codereview.appspot.com/58500043

Committer: Ian Lance Taylor <iant@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants