Skip to content

Commit

Permalink
Use ReadOnlySpan<byte> for parsing FileWatcher responses
Browse files Browse the repository at this point in the history
  • Loading branch information
@jkotas
jkotas committed Sep 19, 2020
1 parent 2a1735b commit bf2faa9
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 102 deletions.
2 changes: 1 addition & 1 deletion src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadDirectoryChangesW.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ internal struct FILE_NOTIFY_INFORMATION

// Note that the file name is not null terminated
internal readonly uint FileNameLength;
internal readonly char FileName;
// internal readonly char FileName[1];
}

internal enum FileAction : uint
Expand Down
205 changes: 104 additions & 101 deletions src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Win32.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@
using System.ComponentModel;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using System.Threading;
using Microsoft.Win32.SafeHandles;

using FILE_NOTIFY_INFORMATION = Interop.Kernel32.FILE_NOTIFY_INFORMATION;

namespace System.IO
{
public partial class FileSystemWatcher
Expand Down Expand Up @@ -235,7 +238,7 @@ private void ReadDirectoryChangesCallback(uint errorCode, uint numBytes, AsyncRe
}
else
{
ParseEventBufferAndNotifyForEach(state.Buffer, numBytes);
ParseEventBufferAndNotifyForEach(new ReadOnlySpan<byte>(state.Buffer, 0, (int)numBytes));
}
}
finally
Expand All @@ -245,118 +248,118 @@ private void ReadDirectoryChangesCallback(uint errorCode, uint numBytes, AsyncRe
}
}

private unsafe void ParseEventBufferAndNotifyForEach(byte[] buffer, uint numBytes)
private unsafe void ParseEventBufferAndNotifyForEach(ReadOnlySpan<byte> buffer)
{
Debug.Assert(buffer != null);
Debug.Assert(buffer.Length > 0);
Debug.Assert(numBytes <= buffer.Length);

numBytes = Math.Min(numBytes, (uint)buffer.Length);
ReadOnlySpan<char> oldName = ReadOnlySpan<char>.Empty;

fixed (byte* b = buffer)
// Parse each event from the buffer and notify appropriate delegates
while (true)
{
byte* pBufferLimit = b + numBytes;

Interop.Kernel32.FILE_NOTIFY_INFORMATION* info = (Interop.Kernel32.FILE_NOTIFY_INFORMATION*)b;

ReadOnlySpan<char> oldName = ReadOnlySpan<char>.Empty;
// Validate the data we received in case it's corrupted. This can happen
// if we are watching files over the network with a poor connection.

// Parse each event from the buffer and notify appropriate delegates
do
// Verify the info object is within the expected bounds
if (sizeof(FILE_NOTIFY_INFORMATION) > (uint)buffer.Length)
{
// Validate the data we received in case it's corrupted. This can happen
// if we are watching files over the network with a poor connection.
// Defensive check. We have not observed this corruption in practice.
Debug.Assert(false);
break;
}
ref readonly FILE_NOTIFY_INFORMATION info = ref MemoryMarshal.AsRef<FILE_NOTIFY_INFORMATION>(buffer);

// Verify the info object is within the expected bounds
if (info < b || (((byte*)info) + sizeof(Interop.Kernel32.FILE_NOTIFY_INFORMATION)) > pBufferLimit)
{
if (info.FileNameLength > (uint)buffer.Length - sizeof(FILE_NOTIFY_INFORMATION))
{
// We have observed this corruption in practice (https://github.com/dotnet/runtime/issues/40412)
break;
}
ReadOnlySpan<char> fileName = MemoryMarshal.Cast<byte, char>(buffer.Slice(sizeof(FILE_NOTIFY_INFORMATION), (int)info.FileNameLength));

// A slightly convoluted piece of code follows. Here's what's happening:
//
// We wish to collapse the poorly done rename notifications from the
// ReadDirectoryChangesW API into a nice rename event. So to do that,
// it's assumed that a FILE_ACTION_RENAMED_OLD_NAME will be followed
// immediately by a FILE_ACTION_RENAMED_NEW_NAME in the buffer, which is
// all that the following code is doing.
//
// On a FILE_ACTION_RENAMED_OLD_NAME, it asserts that no previous one existed
// and saves its name. If there are no more events in the buffer, it'll
// assert and fire a RenameEventArgs with the Name field null.
//
// If a NEW_NAME action comes in with no previous OLD_NAME, we assert and fire
// a rename event with the OldName field null.
//
// If the OLD_NAME and NEW_NAME actions are indeed there one after the other,
// we'll fire the RenamedEventArgs normally and clear oldName.
//
// If the OLD_NAME is followed by another action, we assert and then fire the
// rename event with the Name field null and then fire the next action.
//
// In case it's not a OLD_NAME or NEW_NAME action, we just fire the event normally.
//
// (Phew!)

switch (info.Action)
{
case Interop.Kernel32.FileAction.FILE_ACTION_RENAMED_OLD_NAME:
// Action is renamed from, save the name of the file
oldName = fileName;
break;
}

// Verify the file path is within the bounds
byte* pFileEnd = ((byte*)&(info->FileName)) + info->FileNameLength;
if (pFileEnd < &(info->FileName) || pFileEnd > pBufferLimit)
{
case Interop.Kernel32.FileAction.FILE_ACTION_RENAMED_NEW_NAME:
// oldName may be empty if we didn't receive FILE_ACTION_RENAMED_OLD_NAME first
NotifyRenameEventArgs(
WatcherChangeTypes.Renamed,
fileName,
oldName);
oldName = ReadOnlySpan<char>.Empty;
break;
}

// A slightly convoluted piece of code follows. Here's what's happening:
//
// We wish to collapse the poorly done rename notifications from the
// ReadDirectoryChangesW API into a nice rename event. So to do that,
// it's assumed that a FILE_ACTION_RENAMED_OLD_NAME will be followed
// immediately by a FILE_ACTION_RENAMED_NEW_NAME in the buffer, which is
// all that the following code is doing.
//
// On a FILE_ACTION_RENAMED_OLD_NAME, it asserts that no previous one existed
// and saves its name. If there are no more events in the buffer, it'll
// assert and fire a RenameEventArgs with the Name field null.
//
// If a NEW_NAME action comes in with no previous OLD_NAME, we assert and fire
// a rename event with the OldName field null.
//
// If the OLD_NAME and NEW_NAME actions are indeed there one after the other,
// we'll fire the RenamedEventArgs normally and clear oldName.
//
// If the OLD_NAME is followed by another action, we assert and then fire the
// rename event with the Name field null and then fire the next action.
//
// In case it's not a OLD_NAME or NEW_NAME action, we just fire the event normally.
//
// (Phew!)

ReadOnlySpan<char> fileName = new ReadOnlySpan<char>(&(info->FileName), (int)info->FileNameLength / sizeof(char));

switch (info->Action)
{
case Interop.Kernel32.FileAction.FILE_ACTION_RENAMED_OLD_NAME:
// Action is renamed from, save the name of the file
oldName = fileName;
break;
case Interop.Kernel32.FileAction.FILE_ACTION_RENAMED_NEW_NAME:
// oldName may be empty if we didn't receive FILE_ACTION_RENAMED_OLD_NAME first
NotifyRenameEventArgs(
WatcherChangeTypes.Renamed,
fileName,
oldName);
default:
if (!oldName.IsEmpty)
{
// Previous FILE_ACTION_RENAMED_OLD_NAME with no new name
NotifyRenameEventArgs(WatcherChangeTypes.Renamed, ReadOnlySpan<char>.Empty, oldName);
oldName = ReadOnlySpan<char>.Empty;
break;
default:
if (!oldName.IsEmpty)
{
// Previous FILE_ACTION_RENAMED_OLD_NAME with no new name
NotifyRenameEventArgs(WatcherChangeTypes.Renamed, ReadOnlySpan<char>.Empty, oldName);
oldName = ReadOnlySpan<char>.Empty;
}

switch (info->Action)
{
case Interop.Kernel32.FileAction.FILE_ACTION_ADDED:
NotifyFileSystemEventArgs(WatcherChangeTypes.Created, fileName);
break;
case Interop.Kernel32.FileAction.FILE_ACTION_REMOVED:
NotifyFileSystemEventArgs(WatcherChangeTypes.Deleted, fileName);
break;
case Interop.Kernel32.FileAction.FILE_ACTION_MODIFIED:
NotifyFileSystemEventArgs(WatcherChangeTypes.Changed, fileName);
break;
default:
Debug.Fail($"Unknown FileSystemEvent action type! Value: {info->Action}");
break;
}

break;
}
}

switch (info.Action)
{
case Interop.Kernel32.FileAction.FILE_ACTION_ADDED:
NotifyFileSystemEventArgs(WatcherChangeTypes.Created, fileName);
break;
case Interop.Kernel32.FileAction.FILE_ACTION_REMOVED:
NotifyFileSystemEventArgs(WatcherChangeTypes.Deleted, fileName);
break;
case Interop.Kernel32.FileAction.FILE_ACTION_MODIFIED:
NotifyFileSystemEventArgs(WatcherChangeTypes.Changed, fileName);
break;
default:
Debug.Fail($"Unknown FileSystemEvent action type! Value: {info.Action}");
break;
}

info = info->NextEntryOffset == 0 ? null : (Interop.Kernel32.FILE_NOTIFY_INFORMATION*)((byte*)info + info->NextEntryOffset);
} while (info != null);
break;
}

if (!oldName.IsEmpty)
if (info.NextEntryOffset == 0)
{
// Previous FILE_ACTION_RENAMED_OLD_NAME with no new name
NotifyRenameEventArgs(WatcherChangeTypes.Renamed, ReadOnlySpan<char>.Empty, oldName);
break;
}
} // fixed()

if (info.NextEntryOffset > (uint)buffer.Length)
{
// Defensive check. We have not observed this corruption in practice.
Debug.Assert(false);
break;
}

buffer = buffer.Slice((int)info.NextEntryOffset);
}

if (!oldName.IsEmpty)
{
// Previous FILE_ACTION_RENAMED_OLD_NAME with no new name
NotifyRenameEventArgs(WatcherChangeTypes.Renamed, ReadOnlySpan<char>.Empty, oldName);
}
}

/// <summary>
Expand All @@ -383,7 +386,7 @@ internal AsyncReadState(int session, byte[] buffer, SafeFileHandle handle, Threa
internal byte[] Buffer { get; }
internal SafeFileHandle DirectoryHandle { get; }
internal ThreadPoolBoundHandle ThreadPoolBinding { get; }
internal PreAllocatedOverlapped? PreAllocatedOverlapped { get; set; }
internal PreAllocatedOverlapped? PreAllocatedOverlapped { get; set; }
internal WeakReference<FileSystemWatcher> WeakWatcher { get; }
}
}
Expand Down

0 comments on commit bf2faa9

Please sign in to comment.