dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.96k stars 4.65k forks source link

[API] FileSystemEventArgs extended with NotifyFilters information #30745

Closed Therzok closed 4 days ago

Therzok commented 5 years ago

Currently, the FileSystemWatcher interface does not allow us to check what kind of notification filter triggered the event.

Current status

var watcher = new FileSystemWatcher(path)
{
    NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.FileName | NotifyFilters.DirectoryName,
}
watcher.Created += (o, args) => Console.WriteLine(args.FullPath); // Is this a directory or a file?

Thus, we're left with two options on processing the events:

  1. Register a separate FileSystemWatcher for each known matrix of change types. In this scenario, we would have to create one watcher for filenames, one watcher for directory names.
var fileWatcher = new FileSystemWatcher(path)
{
    NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.FileName,
}
fileWatcher.Created += (o, args) => Console.WriteLine("File created: " + args.FullPath);

var directoryWatcher = new FileSystemWatcher(path)
{
    NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.DirectoryName,
}
directoryWatcher.Created += (o, args) => Console.WriteLine("Dir created: " + args.FullPath);

❗️ This can lead to problems with processing the data. For macOS, we'd have events processed serially, so no problem. But for Linux, we could have events processed out of order, due to each FileSystemWatcher running on its own Task.

  1. Do an IO probe right after.
    var watcher = new FileSystemWatcher(path)
    {
    NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.FileName | NotifyFilters.DirectoryName,
    }
    watcher.Created += (o, args) =>
    {
    var prefix = Directory.Exists(args.FullPath) ? "Dir created: " : "File created: ";
    Console.WriteLine(prefix + args.FullPath);
    }

Rationale and Usage

Given the constraints above could hit performance for various reasons, I would like to suggest introducing a NotifyFilter field on the FileSystemEventArgs class.

namespace System.IO
{
    class FileSystemEventArgs
    {
        /// <summary>
        /// Contains the notification filter values that triggered this event.
        /// </summary>
        public NotifyFilters NotifyFilter { get; }
    } 
}

This would allow us to avoid doing extra work at implementation levels, knowing that an event is associated with a directory via DirectoryName filter or just being able to check which kind of notification an event is for finer grained control without having to create multiple watchers.

stephentoub commented 5 years ago

How do you propose this be implemented?

scalablecory commented 5 years ago

Unfortunately ReadDirectoryChangesW does not expose this information. I don't know how it could be implemented reliably and without large overhead.

stephentoub commented 5 years ago

Exactly.

Therzok commented 5 years ago

Windows

Interesting, did not know that detail about the windows side of things, should've done better initial research.

When is .netcore going to run on Windows 10+ only? 😅 ReadDirectoryChangesExW has file information, but I'm unsure whether this just reports changed values based on the notify filter or all the values - not all the docs mention that.

If it reports actual values, then this API is not helpful at all, because the watcher would have to keep track of the file metadata internally to be able to report deltas - again, a no-go. Another concern would be having fewer events buffered (larger buffer struct size).

I guess there is no real solution for Windows as-is, with the current native APIs.

Unix

macOS: FSEventStream has the required flags passed over on the callback.

Linux: inotify has the required info

End note

I would be happy if we could at least provide whether the changed item is a file or a directory. But that requires bumping the minimum OS requirement to get FileAttributes access and increasing the overhead of information passed by the OS, decreasing the buffer element capacity.

I feel like a dirty hack that would suffice:

    class FileSystemEventArgs
    {
        /// <summary>
        /// Gets whether the notification was for a file or a directory, based on 
        /// </summary>
        public NotificationKind Kind { get; }
    } 

    // Or anything that already exists?
    enum NotificationKind
    {
        Unknown,
        File,
        Directory
    }

And that value would be File/Directory on Win10+, and Unknown on older OS.

Not sure there is a solution given cross-platform constraints, as Windows does not have an easy way to query this information.

Problem this was trying to solve

In VSMac, we have a global file system watcher over the projects, and we are receiving events for IO being done. Being able to differentiate between directories and files has some performance benefits at logic level, as we can dispatch various handlers based on the notification kind (file/directory).

A quick workaround was to create 2 filesystemwatchers, one which monitors files, one which monitors directories and handle them differently. I guess the next step is re-implementing the native watcher with more fine grained control.

carlossanlop commented 4 years ago

Triage - We can consider returning file attributes, but it would have to be opt-in:

dotnet-policy-service[bot] commented 2 weeks ago

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

dotnet-policy-service[bot] commented 4 days ago

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.