Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annotate System.IO.FileSystem.Watcher for nullable reference types #456

Merged
merged 1 commit into from
Dec 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/libraries/Common/src/System/IO/PathInternal.Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

#nullable enable
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;

namespace System.IO
Expand Down Expand Up @@ -74,7 +75,7 @@ private static bool EndsWithPeriodOrSpace(string path)
/// away from paths during normalization, but if we see such a path at this point it should be
/// normalized and has retained the final characters. (Typically from one of the *Info classes)
/// </summary>
/// TODO: add atribute [return: NotNullIfNotNull("path")]
[return: NotNullIfNotNull("path")]
internal static string? EnsureExtendedPrefixIfNeeded(string? path)
{
if (path != null && (path.Length >= MaxShortPath || EndsWithPeriodOrSpace(path)))
Expand Down
5 changes: 3 additions & 2 deletions src/libraries/Common/src/System/Text/ValueUtf8Converter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable enable
using System.Buffers;

namespace System.Text
Expand All @@ -13,7 +14,7 @@ namespace System.Text
/// </summary>
internal ref struct ValueUtf8Converter
{
private byte[] _arrayToReturnToPool;
private byte[]? _arrayToReturnToPool;
private Span<byte> _bytes;

public ValueUtf8Converter(Span<byte> initialBuffer)
Expand All @@ -40,7 +41,7 @@ public Span<byte> ConvertAndTerminateString(ReadOnlySpan<char> value)

public void Dispose()
{
byte[] toReturn = _arrayToReturnToPool;
byte[]? toReturn = _arrayToReturnToPool;
if (toReturn != null)
{
_arrayToReturnToPool = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ public ErrorEventArgs(System.Exception exception) { }
public delegate void ErrorEventHandler(object sender, System.IO.ErrorEventArgs e);
public partial class FileSystemEventArgs : System.EventArgs
{
public FileSystemEventArgs(System.IO.WatcherChangeTypes changeType, string directory, string name) { }
public FileSystemEventArgs(System.IO.WatcherChangeTypes changeType, string directory, string? name) { }
public System.IO.WatcherChangeTypes ChangeType { get { throw null; } }
public string FullPath { get { throw null; } }
public string Name { get { throw null; } }
public string? Name { get { throw null; } }
}
public delegate void FileSystemEventHandler(object sender, System.IO.FileSystemEventArgs e);
public partial class FileSystemWatcher : System.ComponentModel.Component, System.ComponentModel.ISupportInitialize
Expand All @@ -33,13 +33,13 @@ public FileSystemWatcher(string path, string filter) { }
public int InternalBufferSize { get { throw null; } set { } }
public System.IO.NotifyFilters NotifyFilter { get { throw null; } set { } }
public string Path { get { throw null; } set { } }
public override System.ComponentModel.ISite Site { get { throw null; } set { } }
public System.ComponentModel.ISynchronizeInvoke SynchronizingObject { get { throw null; } set { } }
public event System.IO.FileSystemEventHandler Changed { add { } remove { } }
public event System.IO.FileSystemEventHandler Created { add { } remove { } }
public event System.IO.FileSystemEventHandler Deleted { add { } remove { } }
public event System.IO.ErrorEventHandler Error { add { } remove { } }
public event System.IO.RenamedEventHandler Renamed { add { } remove { } }
public override System.ComponentModel.ISite? Site { get { throw null; } set { } }
public System.ComponentModel.ISynchronizeInvoke? SynchronizingObject { get { throw null; } set { } }
public event System.IO.FileSystemEventHandler? Changed { add { } remove { } }
public event System.IO.FileSystemEventHandler? Created { add { } remove { } }
public event System.IO.FileSystemEventHandler? Deleted { add { } remove { } }
public event System.IO.ErrorEventHandler? Error { add { } remove { } }
public event System.IO.RenamedEventHandler? Renamed { add { } remove { } }
public void BeginInit() { }
protected override void Dispose(bool disposing) { }
public void EndInit() { }
Expand All @@ -55,8 +55,8 @@ public partial class InternalBufferOverflowException : System.SystemException
{
public InternalBufferOverflowException() { }
protected InternalBufferOverflowException(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { }
public InternalBufferOverflowException(string message) { }
public InternalBufferOverflowException(string message, System.Exception inner) { }
public InternalBufferOverflowException(string? message) { }
public InternalBufferOverflowException(string? message, System.Exception? inner) { }
}
[System.FlagsAttribute]
public enum NotifyFilters
Expand All @@ -72,18 +72,18 @@ public enum NotifyFilters
}
public partial class RenamedEventArgs : System.IO.FileSystemEventArgs
{
public RenamedEventArgs(System.IO.WatcherChangeTypes changeType, string directory, string name, string oldName) : base (default(System.IO.WatcherChangeTypes), default(string), default(string)) { }
public RenamedEventArgs(System.IO.WatcherChangeTypes changeType, string directory, string? name, string? oldName) : base (default(System.IO.WatcherChangeTypes), default(string), default(string)) { }
public string OldFullPath { get { throw null; } }
public string OldName { get { throw null; } }
public string? OldName { get { throw null; } }
}
public delegate void RenamedEventHandler(object sender, System.IO.RenamedEventArgs e);
public partial struct WaitForChangedResult
{
private object _dummy;
private int _dummyPrimitive;
public System.IO.WatcherChangeTypes ChangeType { readonly get { throw null; } set { } }
public string Name { readonly get { throw null; } set { } }
public string OldName { readonly get { throw null; } set { } }
public string? Name { readonly get { throw null; } set { } }
public string? OldName { readonly get { throw null; } set { } }
public bool TimedOut { readonly get { throw null; } set { } }
}
[System.FlagsAttribute]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<Nullable>enable</Nullable>
<Configurations>netcoreapp-Debug;netcoreapp-Release</Configurations>
</PropertyGroup>
<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Nullable>enable</Nullable>
<Configurations>netcoreapp-FreeBSD-Debug;netcoreapp-FreeBSD-Release;netcoreapp-Linux-Debug;netcoreapp-Linux-Release;netcoreapp-OSX-Debug;netcoreapp-OSX-Release;netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release</Configurations>
</PropertyGroup>
<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ namespace System.IO
public class FileSystemEventArgs : EventArgs
{
private readonly WatcherChangeTypes _changeType;
private readonly string _name;
private readonly string? _name;
private readonly string _fullPath;

/// <devdoc>
/// Initializes a new instance of the <see cref='System.IO.FileSystemEventArgs'/> class.
/// </devdoc>
public FileSystemEventArgs(WatcherChangeTypes changeType, string directory, string name)
public FileSystemEventArgs(WatcherChangeTypes changeType, string directory, string? name)
{
_changeType = changeType;
_name = name;
Expand All @@ -31,7 +31,7 @@ public FileSystemEventArgs(WatcherChangeTypes changeType, string directory, stri
/// This is like Path.Combine, except without argument validation,
/// and a separator is used even if the name argument is empty.
/// </remarks>
internal static string Combine(string directoryPath, string name)
internal static string Combine(string directoryPath, string? name)
{
bool hasSeparator = false;
if (directoryPath.Length > 0)
Expand Down Expand Up @@ -71,7 +71,7 @@ public string FullPath
/// <devdoc>
/// Gets the name of the affected file or directory.
/// </devdoc>
public string Name
public string? Name
{
get
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ private void StartRaisingEvents()
switch (error.Error)
{
case Interop.Error.EMFILE:
string maxValue = ReadMaxUserLimit(MaxUserInstancesPath);
string? maxValue = ReadMaxUserLimit(MaxUserInstancesPath);
string message = !string.IsNullOrEmpty(maxValue) ?
SR.Format(SR.IOException_INotifyInstanceUserLimitExceeded_Value, maxValue) :
SR.IOException_INotifyInstanceUserLimitExceeded;
Expand Down Expand Up @@ -125,12 +125,12 @@ private void FinalizeDispose()
/// Cancellation for the currently running watch operation.
/// This is non-null if an operation has been started and null if stopped.
/// </summary>
private CancellationTokenSource _cancellation;
private CancellationTokenSource? _cancellation;

/// <summary>Reads the value of a max user limit path from procfs.</summary>
/// <param name="path">The path to read.</param>
/// <returns>The value read, or "0" if a failure occurred.</returns>
private static string ReadMaxUserLimit(string path)
private static string? ReadMaxUserLimit(string path)
{
try { return File.ReadAllText(path).Trim(); }
catch { return null; }
Expand Down Expand Up @@ -301,7 +301,7 @@ internal RunningInstance(
internal void Start()
{
// Schedule a task to read from the inotify queue and process the events.
Task.Factory.StartNew(obj => ((RunningInstance)obj).ProcessEvents(),
Task.Factory.StartNew(obj => ((RunningInstance)obj!).ProcessEvents(),
this, CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default);

// PERF: As needed, we can look into making this use async I/O rather than burning
Expand Down Expand Up @@ -337,7 +337,7 @@ private void AddDirectoryWatch(WatchedDirectory parent, string directoryName)
/// <summary>Adds a watch on a directory to the existing inotify handle.</summary>
/// <param name="parent">The parent directory entry.</param>
/// <param name="directoryName">The new directory path to monitor, relative to the root.</param>
private void AddDirectoryWatchUnlocked(WatchedDirectory parent, string directoryName)
private void AddDirectoryWatchUnlocked(WatchedDirectory? parent, string directoryName)
{
string fullPath = parent != null ? parent.GetPath(false, directoryName) : directoryName;

Expand All @@ -363,7 +363,7 @@ private void AddDirectoryWatchUnlocked(WatchedDirectory parent, string directory
Exception exc;
if (error.Error == Interop.Error.ENOSPC)
{
string maxValue = ReadMaxUserLimit(MaxUserWatchesPath);
string? maxValue = ReadMaxUserLimit(MaxUserWatchesPath);
string message = !string.IsNullOrEmpty(maxValue) ?
SR.Format(SR.IOException_INotifyWatchesUserLimitExceeded_Value, maxValue) :
SR.IOException_INotifyWatchesUserLimitExceeded;
Expand All @@ -374,8 +374,7 @@ private void AddDirectoryWatchUnlocked(WatchedDirectory parent, string directory
exc = Interop.GetExceptionForIoErrno(error, fullPath);
}

FileSystemWatcher watcher;
if (_weakWatcher.TryGetTarget(out watcher))
if (_weakWatcher.TryGetTarget(out FileSystemWatcher? watcher))
{
watcher.OnError(new ErrorEventArgs(exc));
}
Expand All @@ -384,7 +383,7 @@ private void AddDirectoryWatchUnlocked(WatchedDirectory parent, string directory
}

// Then store the path information into our map.
WatchedDirectory directoryEntry;
WatchedDirectory? directoryEntry;
bool isNewDirectory = false;
if (_wdToPathMap.TryGetValue(wd, out directoryEntry))
{
Expand All @@ -396,10 +395,7 @@ private void AddDirectoryWatchUnlocked(WatchedDirectory parent, string directory
// of the world, but there's little that can be done about that.)
if (directoryEntry.Parent != parent)
{
if (directoryEntry.Parent != null)
{
directoryEntry.Parent.Children.Remove (directoryEntry);
}
directoryEntry.Parent?.Children!.Remove (directoryEntry);
directoryEntry.Parent = parent;
if (parent != null)
{
Expand Down Expand Up @@ -452,10 +448,7 @@ private void RemoveWatchedDirectory(WatchedDirectory directoryEntry, bool remove
Debug.Assert (_includeSubdirectories);
lock (SyncObj)
{
if (directoryEntry.Parent != null)
{
directoryEntry.Parent.Children.Remove (directoryEntry);
}
directoryEntry.Parent?.Children!.Remove(directoryEntry);
RemoveWatchedDirectoryUnlocked (directoryEntry, removeInotify);
}
}
Expand Down Expand Up @@ -519,12 +512,12 @@ private void ProcessEvents()
// When cancellation is requested, clear out all watches. This should force any active or future reads
// on the inotify handle to return 0 bytes read immediately, allowing us to wake up from the blocking call
// and exit the processing loop and clean up.
var ctr = _cancellationToken.UnsafeRegister(obj => ((RunningInstance)obj).CancellationCallback(), this);
var ctr = _cancellationToken.UnsafeRegister(obj => ((RunningInstance)obj!).CancellationCallback(), this);
try
{
// Previous event information
ReadOnlySpan<char> previousEventName = ReadOnlySpan<char>.Empty;
WatchedDirectory previousEventParent = null;
WatchedDirectory? previousEventParent = null;
uint previousEventCookie = 0;

// Process events as long as we're not canceled and there are more to read...
Expand All @@ -535,15 +528,15 @@ private void ProcessEvents()
// so as to avoid a rooted cycle that would prevent our processing loop from ever ending
// if the watcher is dropped by the user without being disposed. If we can't get the watcher,
// there's nothing more to do (we can't raise events), so bail.
FileSystemWatcher watcher;
FileSystemWatcher? watcher;
if (!_weakWatcher.TryGetTarget(out watcher))
{
break;
}

uint mask = nextEvent.mask;
ReadOnlySpan<char> expandedName = ReadOnlySpan<char>.Empty;
WatchedDirectory associatedDirectoryEntry = null;
WatchedDirectory? associatedDirectoryEntry = null;

// 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
Expand Down Expand Up @@ -729,8 +722,7 @@ private void ProcessEvents()
}
catch (Exception exc)
{
FileSystemWatcher watcher;
if (_weakWatcher.TryGetTarget(out watcher))
if (_weakWatcher.TryGetTarget(out FileSystemWatcher? watcher))
{
watcher.OnError(new ErrorEventArgs(exc));
}
Expand Down Expand Up @@ -852,32 +844,22 @@ private sealed class WatchedDirectory
{
/// <summary>A StringBuilder cached on the current thread to avoid allocations when possible.</summary>
[ThreadStatic]
private static StringBuilder t_builder;
private static StringBuilder? t_builder;

/// <summary>The parent directory.</summary>
internal WatchedDirectory Parent;
internal WatchedDirectory? Parent;

/// <summary>The watch descriptor associated with this directory.</summary>
internal int WatchDescriptor;

/// <summary>The filename of this directory.</summary>
internal string Name;
internal string? Name;

/// <summary>Child directories of this directory for which we added explicit watches.</summary>
internal List<WatchedDirectory> Children;
internal List<WatchedDirectory>? Children;

/// <summary>Child directories of this directory for which we added explicit watches. This is the same as Children, but ensured to be initialized as non-null.</summary>
internal List<WatchedDirectory> InitializedChildren
{
get
{
if (Children == null)
{
Children = new List<WatchedDirectory> ();
}
return Children;
}
}
internal List<WatchedDirectory> InitializedChildren => Children ??= new List<WatchedDirectory>();

// PERF: Work is being done here proportionate to depth of watch directories.
// If this becomes a bottleneck, we'll need to come up with another mechanism
Expand All @@ -894,14 +876,10 @@ internal List<WatchedDirectory> InitializedChildren
/// <param name="relativeToRoot">Whether to get a path relative to the root directory being watched, or a full path.</param>
/// <param name="additionalName">An additional name to include in the path, relative to this directory.</param>
/// <returns>The computed path.</returns>
internal string GetPath(bool relativeToRoot, string additionalName = null)
internal string GetPath(bool relativeToRoot, string? additionalName = null)
{
// Use our cached builder
StringBuilder builder = t_builder;
if (builder == null)
{
t_builder = builder = new StringBuilder();
}
StringBuilder builder = (t_builder ??= new StringBuilder());
builder.Clear();

// Write the directory's path. Then if an additional filename was supplied, append it
Expand Down
Loading