Merge pull request #47 from Tyriar/40_infinite_linux_symlink

Prevent infinite loop when watching circular symlinks on Linux

Author: implausible

includes/linux/InotifyTree.h

@@ -9,6 +9,7 @@
 #include <sstream>
 #include <vector>
 #include <map>
+#include <unordered_set>
 
 class InotifyTree {
 public:
@@ -32,7 +33,8 @@ class InotifyTree {
       int inotifyInstance,
       InotifyNode *parent,
       std::string directory,
-      std::string name
+      std::string name,
+      ino_t inodeNumber
     );
 
     void addChild(std::string name);
@@ -61,6 +63,7 @@ class InotifyTree {
     std::map<std::string, InotifyNode *> *mChildren;
     std::string mDirectory;
     std::string mFullPath;
+    ino_t mInodeNumber;
     const int mInotifyInstance;
     std::string mName;
     InotifyNode *mParent;
@@ -72,10 +75,13 @@ class InotifyTree {
   void setError(std::string error);
   void addNodeReferenceByWD(int watchDescriptor, InotifyNode *node);
   void removeNodeReferenceByWD(int watchDescriptor);
+  bool addInode(ino_t inodeNumber);
+  void removeInode(ino_t inodeNumber);
 
   std::string mError;
   const int mInotifyInstance;
   std::map<int, InotifyNode *> *mInotifyNodeByWatchDescriptor;
+  std::unordered_set<ino_t> inodes;
   InotifyNode *mRoot;
 
   friend class InotifyNode;

js/spec/index-spec.js

@@ -418,6 +418,28 @@ describe('Node Sentinel File Watcher', function() {
         .then(done, () =>
           watch.stop().then((err) => done.fail(err)));
     });
+
+    it('does not loop endlessly when watching directories with recursive symlinks', (done) => {
+      fse.mkdirSync(path.join(workDir, 'test'));
+      fse.symlinkSync(path.join(workDir, 'test'), path.join(workDir, 'test', 'link'));
+
+      let watch;
+
+      return nsfw(
+        workDir,
+        () => {},
+        { debounceMS: DEBOUNCE, errorCallback() {} }
+      )
+        .then(_w => {
+          watch = _w;
+          return watch.start();
+        })
+        .then(() => {
+          return watch.stop();
+        })
+        .then(done, () =>
+          watch.stop().then((err) => done.fail(err)));
+    });
   });
 
   describe('Errors', function() {

src/linux/InotifyTree.cpp

@@ -18,12 +18,20 @@ InotifyTree::InotifyTree(int inotifyInstance, std::string path):
     watchName = path.substr(location + 1);
   }
 
+  struct stat file;
+  if (stat((directory + "/" + watchName).c_str(), &file) < 0) {
+    mRoot = NULL;
+    return;
+  }
+
+  addInode(file.st_ino);
   mRoot = new InotifyNode(
     this,
     mInotifyInstance,
     NULL,
     directory,
-    watchName
+    watchName,
+    file.st_ino
   );
 
   if (
@@ -114,6 +122,14 @@ void InotifyTree::setError(std::string error) {
   mError = error;
 }
 
+bool InotifyTree::addInode(ino_t inodeNumber) {
+  return inodes.insert(inodeNumber).second;
+}
+
+void InotifyTree::removeInode(ino_t inodeNumber) {
+  inodes.erase(inodeNumber);
+}
+
 InotifyTree::~InotifyTree() {
   if (isRootAlive()) {
     delete mRoot;
@@ -128,9 +144,11 @@ InotifyTree::InotifyNode::InotifyNode(
   int inotifyInstance,
   InotifyNode *parent,
   std::string directory,
-  std::string name
+  std::string name,
+  ino_t inodeNumber
 ):
   mDirectory(directory),
+  mInodeNumber(inodeNumber),
   mInotifyInstance(inotifyInstance),
   mName(name),
   mParent(parent),
@@ -170,7 +188,8 @@ InotifyTree::InotifyNode::InotifyNode(
 
     if (
       stat(filePath.c_str(), &file) < 0 ||
-      !S_ISDIR(file.st_mode)
+      !S_ISDIR(file.st_mode) ||
+      !mTree->addInode(file.st_ino) // Skip this inode if already watching
     ) {
       continue;
     }
@@ -180,7 +199,8 @@ InotifyTree::InotifyNode::InotifyNode(
       mInotifyInstance,
       this,
       mFullPath,
-      fileName
+      fileName,
+      file.st_ino
     );
 
     if (child->isAlive()) {
@@ -198,6 +218,8 @@ InotifyTree::InotifyNode::InotifyNode(
 }
 
 InotifyTree::InotifyNode::~InotifyNode() {
+  mTree->removeInode(mInodeNumber);
+
   if (mWatchDescriptorInitialized) {
     inotify_rm_watch(mInotifyInstance, mWatchDescriptor);
     mTree->removeNodeReferenceByWD(mWatchDescriptor);
@@ -211,21 +233,26 @@ InotifyTree::InotifyNode::~InotifyNode() {
 }
 
 void InotifyTree::InotifyNode::addChild(std::string name) {
-  InotifyNode *child = new InotifyNode(
-    mTree,
-    mInotifyInstance,
-    this,
-    mFullPath,
-    name
-  );
+  struct stat file;
 
-  if (
-    child->isAlive() &&
-    child->inotifyInit()
-  ) {
-    (*mChildren)[name] = child;
-  } else {
-    delete child;
+  if (stat(createFullPath(mFullPath, name).c_str(), &file) >= 0 && mTree->addInode(file.st_ino)) {
+    InotifyNode *child = new InotifyNode(
+      mTree,
+      mInotifyInstance,
+      this,
+      mFullPath,
+      name,
+      file.st_ino
+    );
+
+    if (
+      child->isAlive() &&
+      child->inotifyInit()
+    ) {
+      (*mChildren)[name] = child;
+    } else {
+      delete child;
+    }
   }
 }