Bug 5747 - FileSystemWatcher returns invalid paths for events from sub-directories on MacOS X
Summary: FileSystemWatcher returns invalid paths for events from sub-directories on Ma...
Status: RESOLVED FIXED
Alias: None
Product: Class Libraries
Classification: Mono
Component: System ()
Version: master
Hardware: Macintosh Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2012-06-19 10:08 UTC by Alex
Modified: 2013-07-12 06:06 UTC (History)
5 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
test code (1.78 KB, text/plain)
2012-06-19 10:13 UTC, Alex
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on GitHub or Developer Community with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Alex 2012-06-19 10:08:07 UTC
FileSystemWatcher on MacOS X 10.7 returns invalid path for events from sub-folders.

When we create a folder structure like this:
dir1
   subdir
dir2

we receive events:
created folder /dir1
created folder /subdir
created folder /dir2

but should recieve
created folder /dir1
created folder /dir1/subdir
created folder /dir2


Test code:

   public static void Main(string[] args)
        {
            var tempPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
            Directory.CreateDirectory(tempPath);

            var fsw = new FileSystemWatcher(tempPath,"*");
            fsw.IncludeSubdirectories = true;
            fsw.NotifyFilter = NotifyFilters.DirectoryName | NotifyFilters.FileName |
                NotifyFilters.CreationTime | NotifyFilters.Attributes | NotifyFilters.LastWrite | NotifyFilters.Size;           

            fsw.Created += FswOnChanged;
            fsw.Changed += FswOnChanged;
            fsw.EnableRaisingEvents = true;
    
            Console.WriteLine("Created watcher");
            
            // getting watcher type:
            FieldInfo fieldInfo = typeof(FileSystemWatcher).GetField("watcher",
                BindingFlags.Static | BindingFlags.NonPublic);
            object watcher = fieldInfo.GetValue(null);
            Console.WriteLine("watcher type: {0}", watcher.GetType());


            Thread.Sleep(1000);

            // creating folder and files

            Directory.CreateDirectory(Path.Combine(tempPath, "dir1"));
			Thread.Sleep(1000);
	        Directory.CreateDirectory(Path.Combine(tempPath, "dir1","subdir"));
            Directory.CreateDirectory(Path.Combine(tempPath, "dir2"));

    

            Thread.Sleep(1000);

            Directory.Delete(tempPath, true);

        }

        private static void FswOnChanged(object sender, FileSystemEventArgs fileSystemEventArgs)
        {
            Console.WriteLine("Changed path: '{0}'", fileSystemEventArgs.FullPath);
        }

Output:

Created watcher
watcher type: System.IO.KeventWatcher
Changed path: '/var/folders/rp/m13g80ks27947q64hk3f93100000gp/T/f856c44d-f751-4546-bdf3-3550993b385f/dir1'
Changed path: '/var/folders/rp/m13g80ks27947q64hk3f93100000gp/T/f856c44d-f751-4546-bdf3-3550993b385f/subdir'
Changed path: '/var/folders/rp/m13g80ks27947q64hk3f93100000gp/T/f856c44d-f751-4546-bdf3-3550993b385f/dir2' 


Root cause seems to be in System.IO.KeventWatcher (in https://github.com/mono/mono/tree/master/mcs/class/System/System.IO )

In method ProcessEvent file name without a path is passed to FileSystemWatcher.DispatchEvents  

foreach (KeventFileData entry in data.DirEntries.Values) { 
        if (!File.Exists (entry.fsi.FullName) && !Directory.Exists (entry.fsi.FullName)) {
            filename = entry.fsi.Name; //  <---- probably should be entry.fsi.FullName here ----
            fa = FileAction.Removed;
            data.DirEntries.Remove (entry.fsi.FullName);
            PostEvent(filename, fsw, fa, changedFsi);  // This calls FileSystemWatcher.DispatchEvents with file name instead of file path
            break;
        }
}

In DefaultWatcher, full file path is passed to FileSystemWatcher.DispatchEvents
Comment 1 Alex 2012-06-19 10:13:43 UTC
Created attachment 2084 [details]
test code
Comment 2 Robert Wilkens 2012-06-19 18:26:40 UTC
Just wanted to write to verify i can confirm this on my mac.  I might look into it if i have enough time tonight.
Comment 3 Robert Wilkens 2012-06-19 20:25:51 UTC
I have a fix for this but instead of getting 

but should recieve
created folder /dir1
created folder /dir1/subdir
created folder /dir2

we are actually getting the full temp paths up to including /dir1, /dir1/subdir, and /dir2 in my patch.

For example
$ mono test1.exe
Created watcher
watcher type: System.IO.KeventWatcher
Changed path: '/var/folders/6l/cf992gpx35b74rkt63jftytr0000gn/T/7fab769b-e8e3-4669-a74d-740e7997f9d5/dir1'
Changed path: '/var/folders/6l/cf992gpx35b74rkt63jftytr0000gn/T/7fab769b-e8e3-4669-a74d-740e7997f9d5/dir1/subdir'
Changed path: '/var/folders/6l/cf992gpx35b74rkt63jftytr0000gn/T/7fab769b-e8e3-4669-a74d-740e7997f9d5/dir2'

I noticed that this matches the Linux behavior, so I presume it is accurate.

I am not sure about creating platform specific unit tests, but I will at minimum submit the patch on github hopefully later tonight.
Comment 4 Robert Wilkens 2012-06-19 20:55:08 UTC
Fix Committed to my local Mono fork:

https://github.com/robwilkens/mono/commit/44addb3224849a66be6e186708a24d52d00e8066

And pull request submitted: 

https://github.com/mono/mono/pull/334

No guarantee my fix will be accepted, but at least i gave it a try and it seemed to work for me.
Comment 5 Robert Wilkens 2012-06-26 06:11:54 UTC
This was merged into github this morning, which i believe is the 'trunk' you speak of in version, just have to pull in latest updates and rebuild now and this should be fixed if i did it right.
Comment 6 Miguel de Icaza [MSFT] 2012-06-29 15:17:00 UTC
We had to revert this patch, as it breaks for users using XSP.

Bojan has more details.
Comment 7 Robert Wilkens 2012-06-29 15:19:49 UTC
Hence "if i did it right" :-).  I apologize.  I don't know how to set up XSP with Apache or i'd test it myself.  I'll leave this problem alone.
Comment 8 Miguel de Icaza [MSFT] 2012-06-29 15:24:42 UTC
XSP is a self-contained executable, so all you do is type "xsp" on a directory, change some files and apparently it is not reloding them.
Comment 9 Robert Wilkens 2012-06-29 15:31:46 UTC
I will look into it, then, unless someone else does it first.  I'm moving this to the front of my priority queue.
Comment 10 Robert Wilkens 2012-06-29 17:05:19 UTC
Without even looking at the code to xsp, i thought of one reason why it might fail --- but i will test it.  If interested: I may not have considered that / may already be in path we are looking at, in which case, the +1 would move us beyond the first character of the filename.
Comment 11 Robert Wilkens 2012-06-29 17:47:53 UTC
I've verified that if the fullpath ends in / (something which is optional but valid) my patch would have failed and made things worse.  I'm testing a fix for that.  That's my best guess as to why it was failing.

Unfortunately, I am having difficulty testing xsp because when i ran xsp it didn't appear to use my development version of mono (I'm not sure - I put some debug Console.WriteLine messages in and they were never output, plus I couldn't make it fail.
Comment 12 Robert Wilkens 2012-06-29 20:42:09 UTC
Ok, Tried again, but couldn't test against XSP:

https://github.com/mono/mono/pull/361

Perhaps if someone else could test before committing (as i suggest in pull request) it might be wise.

I tested against the above source code modified to test both with and without trailing '/' mark.  Both ways seemed to work this time.
Comment 13 Bojan Rajkovic [MSFT] 2012-06-29 23:56:02 UTC
Hi guys,

A bit more information (and I haven't tested the patch yet, on my Windows box now), but the issue is that FileSystemWatcher (as used in XSP on OS X) breaks with this set of patches--it never gets change notifications processed properly, so views are never recompiled. I think this might be an interaction between HFS+ and that code. As soon as I reboot back to OS X, I'll test the patch in PR #361 and comment on the pull request itself.

Cheers,
Bojan
Comment 14 Robert Wilkens 2012-06-30 00:12:53 UTC
I temporarily CLOSED the pull request because i do not understand: Is this happening on Windows?  Is it happening on Mac?  Or is it happening when a Mac is booted into Windows?  If somehow this code is being executed in Windows, and it should not be as i understand it, then the directory separator check is still wrong.

I was under the impression that this code was only executed on OS X.

The code did have a flaw in it, and i fixed it in PR #361.  That could have caused the problem, but perhaps there is something i do not understand yet.

If you want me to re-open pull request just let me know.

Thanks!
-rob
Comment 15 Robert Wilkens 2012-06-30 00:17:13 UTC
I've gathererd i mis-read your message and re-opened the pull request.  There was something wrong with the old fix which this one corrects.  Try it and let me know if you get a chance.
Comment 16 Robert Wilkens 2012-06-30 11:07:36 UTC
Please understand the reason the patch was done the way it was done was this: "fsi"(FileSystemInformation) for the changed file, which was the only place i think the full path to the file was kept, was only in KeventWatcher - there was no way to give access to that to FileSystemWatcher because FSW has different 'drivers' depending on the environment.  The MacOS driver is KeventWatcher from what i saw.  FullPath of FileSystemWatcher (the path we're monitoring), though, was available in KeventViewer.  The point of this patch was to extract the subdirectory name by stripping off the fsw FullPath from the FullPath of the FileName, which was in fsi, and returning that (the full sub-path) as the filename as happens on other platforms such as linux.  That is "/path1/path2/two" if you were monitoring "/path1/" without the patch and two was changed you'd get back "two" and with this patch you'll get "path2/two".  With the previous version of this patch, it was possible to get "ath2/two" if we ended the watcher fullpath with "/".  This patch fixes that versus the original patch.

It's possible i overlooked something, still.  I couldn't think of too much, though, and would love to be able to test it in XSP but i can't get it to work with my development version, it always uses system wide installed version.
Comment 17 Robert Wilkens 2012-07-01 00:59:39 UTC
I will work with xsp until i can get it to work correctly, no need for you to test yet, i'll see if i can reproduce properly with and without fix.  As of 2.11.2 which is probably where you noticed it, it seems broken.  I closed the pull request until i can figure out why xsp is not updating.
Comment 18 Robert Wilkens 2012-07-01 08:35:00 UTC
I can now reproduce the problem with xsp, and it happens with this newest pull request (361?) as well give me some time to fix this.
Comment 19 Robert Wilkens 2012-07-01 09:21:22 UTC
There were two places in ProcessEvent() in KeventWatcher.cs where PostEvent was called with a null value for changedFsi which is where my code was failing (when it received a null value).  In both cases, they have a fsi object, but they're passing null anyway -- it looks like an oversight in the coding.  I'm experimenting with passing the right values to PostEvent to see if that corrects it, if not simply checking for null should allow this to at least work at the old level of functionality AND fix this bug report.

Depending on what we're doing today, I might have a fix later today or tomorrow in place and tested.
Comment 20 Robert Wilkens 2012-07-01 09:48:13 UTC
Ok, I tested this against XSP and it seemed to work right:

https://github.com/mono/mono/pull/366

But like i mentioned, i fixed another potential problem in this same pull request, so it was good it was reverted so i could investigate.  

Thanks for finding the problem with xsp and this patch.
Comment 21 Bojan Rajkovic [MSFT] 2012-07-09 15:15:04 UTC
I can confirm that pull request #366 (https://github.com/mono/mono/pull/366) fixes this bug and the XSP regression it caused. As soon as someone pulls it into mono/mono, we can re-close this bug.
Comment 22 wojtuch 2013-07-12 06:06:18 UTC
Hello guys, could you please also check the following situation?:

path
     to
        root
           subdir

is the folder structure at the time I start the FSWatcher


path
     to
        root
           subdir
           subdir2

then I create another subdirectory. Changes in the newly created subdirectory are tracked and returned as expected (as under .net implementation). The problem is that the subdir is totally ignored by the FSWatcher...

I test it under OSX, all Mono versions I've tried by now are affected.