Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(10)

Issue 5418045: code review 5418045: inotify: Fix readEntry hanging and Add/RemoveWatch fail... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 10 months ago by ugorji
Modified:
8 years, 6 months ago
Reviewers:
CC:
golang-dev
Visibility:
Public.

Description

inotify: Fix readEntry hanging and Add/RemoveWatch failing when watched directories deleted and re-created. If a watched directory is deleted and recreated, and watched again, many of the events sent back are hosed. - The event.Name is incomplete, missing the path of the watched directory (you get /a.txt or "") - RemoveWatch thereafter fails with message: invalid argument - Close() doesn't really close, and readEvents hangs forever on syscall.Read(...) Note that the readEvents() hanging was always there, irrespective of the steps to produce issues with Add/RemoveWatch. Fix involves: - use Select to ensure there's data to read, else syscall.Read(...) will keep on waiting, even though intent to close was sent. - Let Close() wait till completely closed (use sync channel with send/acknowledge receipt). - AddWatch should always modify watch, in case watch description from OS changes (e.g. due to move, delete/re-add, etc) Also, added a test for this: TestInotifyDeleteReAdd Fixes issue 2483.

Patch Set 1 #

Patch Set 2 : diff -r 6abf04c86097 https://code.google.com/p/go/ #

Total comments: 1

Patch Set 3 : diff -r 6abf04c86097 https://code.google.com/p/go/ #

Patch Set 4 : diff -r 6abf04c86097 https://code.google.com/p/go/ #

Total comments: 6

Patch Set 5 : diff -r c8f7a5a37d09 https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+156 lines, -10 lines) Patch
M src/pkg/exp/inotify/inotify_linux.go View 1 2 3 4 6 chunks +38 lines, -10 lines 0 comments Download
M src/pkg/exp/inotify/inotify_linux_test.go View 1 2 3 4 2 chunks +118 lines, -0 lines 0 comments Download

Messages

Total messages: 13
ugorji
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
9 years, 10 months ago (2011-11-18 23:30:41 UTC) #1
ugorji
http://codereview.appspot.com/5418045/diff/2001/src/pkg/exp/inotify/inotify_linux.go File src/pkg/exp/inotify/inotify_linux.go (right): http://codereview.appspot.com/5418045/diff/2001/src/pkg/exp/inotify/inotify_linux.go#newcode155 src/pkg/exp/inotify/inotify_linux.go:155: fdset.Bits[w.fd/4] |= 1 << uint(w.fd%4) //do fdset Please look ...
9 years, 10 months ago (2011-11-19 02:28:43 UTC) #2
ugorji
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 10 months ago (2011-11-21 23:25:03 UTC) #3
ugorji
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 10 months ago (2011-11-21 23:25:22 UTC) #4
leczb
On 2011/11/21 23:25:22, ugorji wrote: > Hello mailto:golang-dev@googlegroups.com (cc: mailto:golang-dev@googlegroups.com), > > Please take another ...
9 years, 9 months ago (2011-12-05 15:15:36 UTC) #5
bradfitz
http://codereview.appspot.com/5418045/diff/5002/src/pkg/exp/inotify/inotify_linux.go File src/pkg/exp/inotify/inotify_linux.go (right): http://codereview.appspot.com/5418045/diff/5002/src/pkg/exp/inotify/inotify_linux.go#newcode76 src/pkg/exp/inotify/inotify_linux.go:76: done: make(chan bool), //remove buffer. make sync. this comment ...
9 years, 9 months ago (2011-12-05 16:26:29 UTC) #6
ugorji
Hello golang-dev@googlegroups.com, leczb@google.com, bradfitz@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 9 months ago (2011-12-05 22:48:32 UTC) #7
ugorji
9 years, 9 months ago (2011-12-05 22:50:22 UTC) #8
ugorji
http://codereview.appspot.com/5418045/diff/5002/src/pkg/exp/inotify/inotify_linux.go File src/pkg/exp/inotify/inotify_linux.go (right): http://codereview.appspot.com/5418045/diff/5002/src/pkg/exp/inotify/inotify_linux.go#newcode76 src/pkg/exp/inotify/inotify_linux.go:76: done: make(chan bool), //remove buffer. make sync. On 2011/12/05 ...
9 years, 9 months ago (2011-12-05 22:54:51 UTC) #9
bradfitz
What's the status of this? On Mon, Nov 21, 2011 at 3:25 PM, <ugorji@gmail.com> wrote: ...
9 years, 8 months ago (2012-01-17 21:41:11 UTC) #10
rsc
I think the status is that inotify is in exp so we've been leaving this ...
9 years, 8 months ago (2012-01-17 22:14:23 UTC) #11
rsc
9 years, 3 months ago (2012-06-03 04:40:36 UTC) #12
ugorji
8 years, 6 months ago (2013-03-20 16:12:14 UTC) #13
Message was sent while issue was closed.
*** Abandoned ***
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b