Permalink
Browse files

fs: fix fs.watch() segmentation fault

The binding layer failed to initialize the event string if both UV_RENAME and
UV_CHANGE were set.

Fixes #2287.
  • Loading branch information...
1 parent eef5d32 commit 22c2c34952b22c982aca23d2ce74817d20f4c1cc @bnoordhuis bnoordhuis committed Dec 7, 2011
Showing with 21 additions and 9 deletions.
  1. +21 −9 src/fs_event_wrap.cc
View
@@ -133,18 +133,30 @@ void FSEventWrap::OnEvent(uv_fs_event_t* handle, const char* filename,
assert(wrap->object_.IsEmpty() == false);
+ // We're in a bind here. libuv can set both UV_RENAME and UV_CHANGE but
+ // the Node API only lets us pass a single event to JS land.
+ //
+ // The obvious solution is to run the callback twice, once for each event.
+ // However, since the second event is not allowed to fire if the handle is
+ // closed after the first event, and since there is no good way to detect
+ // closed handles, that option is out.
+ //
+ // For now, ignore the UV_CHANGE event if UV_RENAME is also set. Make the
+ // assumption that a rename implicitly means an attribute change. Not too
+ // unreasonable, right? Still, we should revisit this before v1.0.
@egeozcan

egeozcan Dec 7, 2011

Definitely reasonable. Thanks a lot for the fix.

if (status) {
SetErrno(uv_last_error(uv_default_loop()));
eventStr = String::Empty();
- } else {
- switch (events) {
- case UV_RENAME:
- eventStr = String::New("rename");
- break;
- case UV_CHANGE:
- eventStr = String::New("change");
- break;
- }
+ }
+ else if (events & UV_RENAME) {
+ eventStr = String::New("rename");
+ }
+ else if (events & UV_CHANGE) {
+ eventStr = String::New("change");
+ }
+ else {
+ assert(0 && "bad fs events flag");
+ abort();
}
Local<Value> argv[3] = {

0 comments on commit 22c2c34

Please sign in to comment.