Merge pull request #2587 from sokket/fsw

Adding new Unit Tests to validate the more complex FileSystemWatcher scenarios

Author: stephentoub

src/Common/src/Interop/Linux/libc/Interop.inotify.cs

@@ -53,6 +53,7 @@ internal enum NotifyEvents
             IN_Q_OVERFLOW  = 0x00004000,
             IN_IGNORED     = 0x00008000,
             IN_ONLYDIR     = 0x01000000,
+            IN_DONT_FOLLOW = 0x02000000,
             IN_EXCL_UNLINK = 0x04000000,
             IN_ISDIR       = 0x40000000,
         }

src/Common/src/Interop/Unix/System.IO.Native/Interop.NativeIO.cs

@@ -43,5 +43,8 @@ internal enum FileStatsFlags
 
         [DllImport(Libraries.IOInterop, SetLastError = true)]
         internal static extern int Stat(string path, out FileStats output);
+
+        [DllImport(Libraries.IOInterop, SetLastError = true)]
+        internal static extern int LStat(string path, out FileStats output);
     }
 }

src/Common/src/Interop/Unix/libc/Interop.poll.cs

@@ -0,0 +1,62 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.Runtime.InteropServices;
+
+internal static partial class Interop
+{
+    internal static partial class libc
+    {
+        [Flags]
+        internal enum PollFlags : short
+        {
+            POLLIN      = 0x0001,       /* any readable data available */
+            POLLERR     = 0x0008,       /* some poll error occurred */
+            POLLHUP     = 0x0010,       /* file descriptor was "hung up" */
+            POLLNVAL    = 0x0020,       /* requested events "invalid" */
+        }
+
+        [StructLayout(LayoutKind.Sequential)]
+        internal struct pollfd
+        {
+            internal int fd;            /* file descriptor to poll*/
+            internal PollFlags events;  /* events to poll for */
+            internal PollFlags revents; /* events received from polling */
+        }
+
+        /// <summary>
+        /// Polls a set of file descriptors for signals and returns what signals have been set
+        /// </summary>
+        /// <param name="fds">A pointer to pollfd structs to look for</param>
+        /// <param name="count">The number of entries in fds</param>
+        /// <param name="timeout">The amount of time to wait; -1 for infinite, 0 for immediate return, and a positive number is the number of milliseconds</param>
+        /// <returns>
+        /// Returns a positive number (which is the number of structures with nonzero revent files), 0 for a timeout or no 
+        /// descriptors were ready, or -1 on error.
+        /// </returns>
+        [DllImport(Libraries.Libc, SetLastError = true)]
+        private static unsafe extern int poll(pollfd* fds, uint count, int timeout);
+
+        /// <summary>
+        /// Polls a File Descriptor for the passed in flags.
+        /// </summary>
+        /// <param name="fd">The descriptor to poll</param>
+        /// <param name="flags">The flags to poll for</param>
+        /// <param name="timeout">The amount of time to wait; -1 for infinite, 0 for immediate return, and a positive number is the number of milliseconds</param>
+        /// <param name="resultFlags">The flags that were returned by the poll call</param>
+        /// <returns>
+        /// Returns a positive number (which is the number of structures with nonzero revent files), 0 for a timeout or no 
+        /// descriptors were ready, or -1 on error.
+        /// </returns>
+        internal unsafe static int poll(int fd, PollFlags flags, int timeout, out PollFlags resultFlags)
+        {
+            pollfd pfd = default(pollfd);
+            pfd.fd = fd;
+            pfd.events = flags;
+            int result = poll(&pfd, 1, timeout);
+            resultFlags = pfd.revents;
+            return result;
+        }
+    }
+}

src/Native/System.IO.Native/nativeio.cpp

@@ -10,9 +10,11 @@
 #if HAVE_STAT64
 #    define stat_ stat64
 #    define fstat_ fstat64
+#    define lstat_ lstat64
 #else
 #   define stat_ stat
 #   define fstat_ fstat
+#   define lstat_ lstat
 #endif
 
 static void ConvertFileStats(const struct stat_& src, FileStats* dst)
@@ -59,4 +61,17 @@ extern "C"
 
         return ret; // TODO: errno conversion
     }
+
+    int32_t LStat(const char* path, struct FileStats* output)
+    {
+        struct stat_ result;
+        int ret = lstat_(path, &result);
+
+        if (ret == 0)
+        {
+            ConvertFileStats(result, output);
+        }
+
+        return ret; // TODO: errno conversion
+    }
 }

src/Native/System.IO.Native/nativeio.h

@@ -41,5 +41,12 @@ extern "C"
      * Returns 0 for success, -1 for failure. Sets errno on failure.
      */
     int32_t Stat(const char* path, FileStats* output);
+
+    /**
+    * Get file stats from a full path. Implemented as shim to lstat(2).
+    *
+    * Returns 0 for success, -1 for failure. Sets errno on failure.
+    */
+    int32_t LStat(const char* path, FileStats* output);
 }
 

src/System.IO.FileSystem.Watcher/src/System.IO.FileSystem.Watcher.csproj

@@ -66,6 +66,12 @@
     <Compile Include="$(CommonPath)\Interop\Linux\Interop.Errors.cs">
       <Link>Common\Interop\Linux\Interop.Errors.cs</Link>
     </Compile>
+    <Compile Include="$(CommonPath)\Interop\Unix\libc\Interop.poll.cs">
+      <Link>Common\Interop\Unix\Interop.poll.cs</Link>
+    </Compile>
+    <Compile Include="$(CommonPath)\Interop\Unix\System.IO.Native\Interop.NativeIO.cs">
+      <Link>Common\Interop\Unix\Interop.NativeIO.cs</Link>
+    </Compile>
   </ItemGroup>
   <ItemGroup Condition=" '$(TargetsOSX)' == 'true' ">
     <Compile Include="System\IO\FileSystemWatcher.OSX.cs" />

src/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs

@@ -299,10 +299,20 @@ private WatchedDirectory AddDirectoryWatchUnlocked(WatchedDirectory parent, stri
             {
                 string fullPath = parent != null ? parent.GetPath(false, directoryName) : directoryName;
 
+                // inotify_add_watch will fail if this is a symlink, so check that we didn't get a symlink
+                Interop.NativeIO.FileStats stats = default(Interop.NativeIO.FileStats);
+                if ((Interop.NativeIO.LStat(fullPath, out stats) == 0) &&
+                    ((stats.Mode & (uint)Interop.NativeIO.FileTypes.S_IFMT) == Interop.NativeIO.FileTypes.S_IFLNK))
+                {
+                    return null;
+                }
+
                 // Add a watch for the full path.  If the path is already being watched, this will return 
-                // the existing descriptor.  This works even in the case of a rename.
+                // the existing descriptor.  This works even in the case of a rename. We also add the DONT_FOLLOW
+                // and EXCL_UNLINK flags to keep parity with Windows where we don't pickup symlinks or unlinked
+                // files (which don't exist in Windows)
                 int wd = (int)SysCall(
-                    (fd, path, thisRef) => Interop.libc.inotify_add_watch(fd, path, (uint)thisRef._notifyFilters),
+                    (fd, path, thisRef) => Interop.libc.inotify_add_watch(fd, path, (uint)(thisRef._notifyFilters | Interop.libc.NotifyEvents.IN_DONT_FOLLOW | Interop.libc.NotifyEvents.IN_EXCL_UNLINK)),
                     fullPath,
                     this);
 
@@ -476,7 +486,21 @@ private void ProcessEvents()
                         string expandedName = null;
                         WatchedDirectory associatedDirectoryEntry = null;
 
-                        if (nextEvent.wd != -1) // wd is -1 for events like IN_Q_OVERFLOW that aren't tied to a particular watch descriptor
+                        // An overflow event means that we can't trust our state without restarting since we missed events and 
+                        // some of those events could be a directory create, meaning we wouldn't have added the directory to the 
+                        // watch and would not provide correct data to the caller.
+                        if ((mask & (uint)Interop.libc.NotifyEvents.IN_Q_OVERFLOW) != 0)
+                        {
+                            // Notify the caller of the error and, if the includeSubdirectories flag is set, restart to pick up any
+                            // potential directories we missed due to the overflow.
+                            watcher.NotifyInternalBufferOverflowEvent();
+                            if (_includeSubdirectories)
+                            {
+                                watcher.Restart();
+                            }
+                            break;
+                        }
+                        else
                         {
                             // Look up the directory information for the supplied wd
                             lock (SyncObj)
@@ -543,15 +567,11 @@ private void ProcessEvents()
                         }
 
                         const Interop.libc.NotifyEvents switchMask =
-                            Interop.libc.NotifyEvents.IN_Q_OVERFLOW | Interop.libc.NotifyEvents.IN_IGNORED |
-                            Interop.libc.NotifyEvents.IN_CREATE | Interop.libc.NotifyEvents.IN_DELETE |
+                            Interop.libc.NotifyEvents.IN_IGNORED |Interop.libc.NotifyEvents.IN_CREATE | Interop.libc.NotifyEvents.IN_DELETE |
                             Interop.libc.NotifyEvents.IN_ACCESS | Interop.libc.NotifyEvents.IN_MODIFY | Interop.libc.NotifyEvents.IN_ATTRIB |
                             Interop.libc.NotifyEvents.IN_MOVED_FROM | Interop.libc.NotifyEvents.IN_MOVED_TO;
                         switch ((Interop.libc.NotifyEvents)(mask & (uint)switchMask))
                         {
-                            case Interop.libc.NotifyEvents.IN_Q_OVERFLOW:
-                                watcher.NotifyInternalBufferOverflowEvent();
-                                break;
                             case Interop.libc.NotifyEvents.IN_CREATE:
                                 watcher.NotifyFileSystemEventArgs(WatcherChangeTypes.Created, expandedName);
                                 break;
@@ -573,9 +593,35 @@ private void ProcessEvents()
                                 watcher.NotifyFileSystemEventArgs(WatcherChangeTypes.Changed, expandedName);
                                 break;
                             case Interop.libc.NotifyEvents.IN_MOVED_FROM:
+                                // We need to check if this MOVED_FROM event is standalone - meaning the item was moved out
+                                // of scope. We do this by checking if we are at the end of our buffer (meaning no more events) 
+                                // and if there is data to be read by polling the fd. If there aren't any more events, fire the
+                                // deleted event; if there are more events, handle it via next pass. This adds an additional
+                                // edge case where we get the MOVED_FROM event and the MOVED_TO event hasn't been generated yet
+                                // so we will send a DELETE for this event and a CREATE when the MOVED_TO is eventually processed.
+                                if (_bufferPos == _bufferAvailable)
+                                {
+                                    bool success = false;
+                                    Interop.libc.PollFlags resultFlags;
+                                    _inotifyHandle.DangerousAddRef(ref success);
+                                    Debug.Assert(success, "Failed to add-ref inotify handle");
+                                    int result = Interop.libc.poll(_inotifyHandle.DangerousGetHandle().ToInt32(), Interop.libc.PollFlags.POLLIN, 0, out resultFlags);
+                                    _inotifyHandle.DangerousRelease();
+
+                                    // If we error or don't have any signaled handles, send the deleted event
+                                    if (result <= 0)
+                                    {
+                                        // There isn't any more data in the queue so this is a deleted event
+                                        watcher.NotifyFileSystemEventArgs(WatcherChangeTypes.Deleted, expandedName);
+                                        break;
+                                    }
+                                }
+
+                                // We will set these values if the buffer has more data OR if the poll call tells us that more data is available.
                                 previousEventName = expandedName;
                                 previousEventParent = isDir ? associatedDirectoryEntry : null;
                                 previousEventCookie = nextEvent.cookie;
+
                                 break;
                             case Interop.libc.NotifyEvents.IN_MOVED_TO:
                                 if (previousEventName != null)

src/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs

@@ -229,7 +229,7 @@ private void FileSystemEventCallback(
             FSEventStreamEventId[] eventIds)
         {
             Debug.Assert((numEvents.ToInt32() == eventPaths.Length) && (numEvents.ToInt32() == eventFlags.Length) && (numEvents.ToInt32() == eventIds.Length));
-            
+
             // Since renames come in pairs, when we find the first we need to search for the next one. Once we find it, we'll add it to this
             // list so when the for-loop comes across it, we'll skip it since it's already been processed as part of the original of the pair.
             List<FSEventStreamEventId> handledRenameEvents = null;
@@ -310,19 +310,21 @@ private void FileSystemEventCallback(
                     }
                     else
                     {
-                        // Note: we keep the same order (Create, delete, modify) as *nix but since OS X does some coalescing on it's own, we need to
-                        //       fire multiple events for the same item (potentially).
-                        if (IsFlagSet(eventFlags[i], Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemCreated))
-                        {
-                            // Next look for creates since a create + modification coalesced could confuse apps since a file they haven't heard of yet would get a mod event
-                            NotifyFileSystemEventArgs(WatcherChangeTypes.Created, relativePath);
-                        }
-                        
-                        if (IsFlagSet(eventFlags[i], Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemRemoved))
+                        // OS X is wonky where it can give back kFSEventStreamEventFlagItemCreated and kFSEventStreamEventFlagItemRemoved
+                        // for the same item. The only option we have is to stat and see if the item exists; if so send created, otherwise send deleted.
+                        if ((IsFlagSet(eventFlags[i], Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemCreated)) ||
+                            (IsFlagSet(eventFlags[i], Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemRemoved)))
                         {
-                            NotifyFileSystemEventArgs(WatcherChangeTypes.Deleted, relativePath);
+                            if (DoesItemExist(eventPaths[i], IsFlagSet(eventFlags[i], Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsFile)))
+                            {
+                                NotifyFileSystemEventArgs(WatcherChangeTypes.Created, relativePath);
+                            }
+                            else
+                            {
+                                NotifyFileSystemEventArgs(WatcherChangeTypes.Deleted, relativePath);
+                            }
                         }
-                        
+
                         if (IsFlagSet(eventFlags[i], Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemInodeMetaMod) ||
                             IsFlagSet(eventFlags[i], Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemModified) ||
                             IsFlagSet(eventFlags[i], Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemFinderInfoMod) ||