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.

Sign up for GitHub

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

Jump to bottom

Handle FileNotFound for symlinks when using polling #56915

Merged
merged 3 commits into from
Aug 10, 2021
Merged

Handle FileNotFound for symlinks when using polling #56915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a4457b2
Handle FileNotFound for symlinks when using polling
jozkee Aug 5, 2021
a9ecb2d
Re-enable tests
jozkee Aug 5, 2021
13fed76
Use CTS instead of Task.Wait(TimeSpan) to fix issues in browser.
jozkee Aug 5, 2021
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
20 changes: 16 additions & 4 deletions ...ibraries/Microsoft.Extensions.FileProviders.Physical/src/Internal/FileSystemInfoHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,28 @@ public static bool IsExcluded(FileSystemInfo fileSystemInfo, ExclusionFilters fi
// If file is a link, and link target does not exists, return DateTime.MinValue
// since the link's LastWriteTimeUtc doesn't convey anything for this scenario.
// If file is not a link, return null to inform the caller that file is not a link.
public static DateTime? GetFileLinkTargetLastWriteTimeUtc(FileInfo fileInfo)
public static DateTime? GetFileLinkTargetLastWriteTimeUtc(FileInfo fileInfo, bool isSecondTry = false)
{
#if NETCOREAPP
Debug.Assert(fileInfo.Exists);
if (fileInfo.LinkTarget != null)
{
FileSystemInfo targetInfo = fileInfo.ResolveLinkTarget(returnFinalTarget: true);
if (targetInfo.Exists)
try
{
return targetInfo.LastWriteTimeUtc;
FileSystemInfo targetInfo = fileInfo.ResolveLinkTarget(returnFinalTarget: true);
if (targetInfo.Exists)
{
return targetInfo.LastWriteTimeUtc;
}
}
catch (FileNotFoundException)
{
// The file ceased to exist between LinkTarget and ResolveLinkTarget.
// Try one more time, if it fails again just give up.
if (!isSecondTry)
{
GetFileLinkTargetLastWriteTimeUtc(fileInfo, isSecondTry: true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jozkee I'm missing something, shouldn't this return a value?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😳 I overlooked that too!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be interesting to have an analyzer that flagged ignored return values, although it would be hugely noisy presumably @stephentoub

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danmoseley great catch, I've sent #57136 to address that

}
}

return DateTime.MinValue;
Expand Down
26 changes: 15 additions & 11 deletions src/libraries/Microsoft.Extensions.FileProviders.Physical/tests/PhysicalFileProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1540,7 +1540,6 @@ public void UsePollingFileWatcher_FileWatcherNotNull_ReturnsFalse()
[Theory]
[InlineData(false)]
[InlineData(true)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/56190", TestPlatforms.AnyUnix)]
public async Task UsePollingFileWatcher_UseActivePolling_HasChanged(bool useWildcard)
{
// Arrange
Expand All @@ -1550,25 +1549,27 @@ public async Task UsePollingFileWatcher_UseActivePolling_HasChanged(bool useWild
File.WriteAllText(filePath, "v1.1");

using var provider = new PhysicalFileProvider(root.RootPath) { UsePollingFileWatcher = true, UseActivePolling = true };
IChangeToken token = provider.Watch(useWildcard ? "*" : fileName);
IChangeToken changeToken = provider.Watch(useWildcard ? "*" : fileName);

var tcs = new TaskCompletionSource<object>();
token.RegisterChangeCallback(_ => { tcs.TrySetResult(null); }, null);
var tcs = new TaskCompletionSource<bool>();
changeToken.RegisterChangeCallback(_ => { tcs.TrySetResult(true); }, null);

var cts = new CancellationTokenSource(TimeSpan.FromSeconds(30));
cts.Token.Register(() => tcs.TrySetCanceled());

// Act
await Task.Delay(1000); // Wait a second before writing again, see https://github.com/dotnet/runtime/issues/55951.
File.WriteAllText(filePath, "v1.2");

// Assert
Assert.True(tcs.Task.Wait(TimeSpan.FromSeconds(30)),
Assert.True(await tcs.Task,
$"Change event was not raised - current time: {DateTime.UtcNow:O}, file LastWriteTimeUtc: {File.GetLastWriteTimeUtc(filePath):O}");
}

[Theory]
[InlineData(false)]
[InlineData(true)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/56190", TestPlatforms.AnyUnix)]
public void UsePollingFileWatcher_UseActivePolling_HasChanged_FileDeleted(bool useWildcard)
public async Task UsePollingFileWatcher_UseActivePolling_HasChanged_FileDeleted(bool useWildcard)
{
// Arrange
using var root = new DisposableFileSystem();
Expand All @@ -1578,16 +1579,19 @@ public void UsePollingFileWatcher_UseActivePolling_HasChanged_FileDeleted(bool u

string filter = useWildcard ? "*" : fileName;
using var provider = new PhysicalFileProvider(root.RootPath) { UsePollingFileWatcher = true, UseActivePolling = true };
IChangeToken token = provider.Watch(filter);
IChangeToken changeToken = provider.Watch(filter);

var tcs = new TaskCompletionSource<bool>();
changeToken.RegisterChangeCallback(_ => { tcs.TrySetResult(true); }, null);

var tcs = new TaskCompletionSource<object>();
token.RegisterChangeCallback(_ => { tcs.TrySetResult(null); }, null);
var cts = new CancellationTokenSource(TimeSpan.FromSeconds(30));
cts.Token.Register(() => tcs.TrySetCanceled());

// Act
File.Delete(filePath);

// Assert
Assert.True(tcs.Task.Wait(TimeSpan.FromSeconds(30)),
Assert.True(await tcs.Task,
$"Change event was not raised - current time: {DateTime.UtcNow:O}, file Exists: {File.Exists(filePath)}.");
}

Expand Down
45 changes: 26 additions & 19 deletions ...Microsoft.Extensions.FileProviders.Physical/tests/PhysicalFileProviderTests.netcoreapp.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Primitives;
using Xunit;
Expand All @@ -14,7 +15,6 @@ public partial class PhysicalFileProviderTests
[Theory]
[InlineData(false)]
[InlineData(true)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/56190", TestPlatforms.AnyUnix)]
public async Task UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink(bool useWildcard)
{
// Arrange
Expand All @@ -30,23 +30,25 @@ public async Task UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink
using var provider = new PhysicalFileProvider(rootOfLink.RootPath) { UsePollingFileWatcher = true, UseActivePolling = true };
IChangeToken token = provider.Watch(useWildcard ? "*" : linkName);

var tcs = new TaskCompletionSource();
token.RegisterChangeCallback(_ => { tcs.TrySetResult(); }, null);
var tcs = new TaskCompletionSource<bool>();
token.RegisterChangeCallback(_ => { tcs.TrySetResult(true); }, null);

var cts = new CancellationTokenSource(TimeSpan.FromSeconds(30));
cts.Token.Register(() => tcs.TrySetCanceled());

// Act
await Task.Delay(1000); // Wait a second before writing again, see https://github.com/dotnet/runtime/issues/55951.
File.WriteAllText(filePath, "v1.2");

// Assert
Assert.True(tcs.Task.Wait(TimeSpan.FromSeconds(30)),
Assert.True(await tcs.Task,
$"Change event was not raised - current time: {DateTime.UtcNow:O}, file LastWriteTimeUtc: {File.GetLastWriteTimeUtc(filePath):O}.");
}

[Theory]
[InlineData(false)]
[InlineData(true)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/56190", TestPlatforms.AnyUnix)]
public void UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_TargetNotExists(bool useWildcard)
public async Task UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_TargetNotExists(bool useWildcard)
{
// Arrange
using var rootOfLink = new DisposableFileSystem();
Expand All @@ -59,19 +61,19 @@ public void UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_Targe
IChangeToken token = provider.Watch(useWildcard ? "*" : linkName);

var tcs = new TaskCompletionSource();
token.RegisterChangeCallback(_ => { tcs.TrySetResult(); }, null);
token.RegisterChangeCallback(_ => { Assert.True(false, "Change event was raised when it was not expected."); }, null);

// Assert
Assert.False(tcs.Task.Wait(TimeSpan.FromSeconds(30)),
"Change event was raised when it was not expected.");
var cts = new CancellationTokenSource(TimeSpan.FromSeconds(30));
cts.Token.Register(() => tcs.TrySetCanceled());

await Assert.ThrowsAsync<TaskCanceledException>(() => tcs.Task);
}

[Theory]
[InlineData(false, false)]
[InlineData(false, true)]
[InlineData(true, false)]
[InlineData(true, true)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/56190", TestPlatforms.AnyUnix)]
public async Task UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_TargetChanged(bool useWildcard, bool linkWasBroken)
{
// Arrange
Expand All @@ -96,23 +98,25 @@ public async Task UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink
using var provider = new PhysicalFileProvider(rootOfLink.RootPath) { UsePollingFileWatcher = true, UseActivePolling = true };
IChangeToken token = provider.Watch(filter);

var tcs = new TaskCompletionSource();
token.RegisterChangeCallback(_ => { tcs.TrySetResult(); }, null);
var tcs = new TaskCompletionSource<bool>();
token.RegisterChangeCallback(_ => { tcs.TrySetResult(true); }, null);

var cts = new CancellationTokenSource(TimeSpan.FromSeconds(30));
cts.Token.Register(() => tcs.TrySetCanceled());

// Act - Change link target to file 2.
File.Delete(linkPath);
File.CreateSymbolicLink(linkPath, file2Path);

// Assert - It should report the change regardless of the timestamp being older.
Assert.True(tcs.Task.Wait(TimeSpan.FromSeconds(30)),
Assert.True(await tcs.Task,
$"Change event was not raised - current time: {DateTime.UtcNow:O}, file1 LastWriteTimeUtc: {File.GetLastWriteTimeUtc(file1Path):O}, file2 LastWriteTime: {File.GetLastWriteTimeUtc(file2Path):O}.");
}

[Theory]
[InlineData(false)]
[InlineData(true)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/56190", TestPlatforms.AnyUnix)]
public void UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_TargetDeleted(bool useWildcard)
public async Task UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_TargetDeleted(bool useWildcard)
{
// Arrange
using var rootOfFile = new DisposableFileSystem();
Expand All @@ -129,14 +133,17 @@ public void UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_Targe
using var provider = new PhysicalFileProvider(rootOfLink.RootPath) { UsePollingFileWatcher = true, UseActivePolling = true };
IChangeToken token = provider.Watch(filter);

var tcs = new TaskCompletionSource();
token.RegisterChangeCallback(_ => { tcs.TrySetResult(); }, null);
var tcs = new TaskCompletionSource<bool>();
token.RegisterChangeCallback(_ => { tcs.TrySetResult(true); }, null);

var cts = new CancellationTokenSource(TimeSpan.FromSeconds(30));
cts.Token.Register(() => tcs.TrySetCanceled());

// Act
File.Delete(linkPath);

// Assert
Assert.True(tcs.Task.Wait(TimeSpan.FromSeconds(30)),
Assert.True(await tcs.Task,
$"Change event was not raised - current time: {DateTime.UtcNow:O}, file LastWriteTimeUtc: {File.GetLastWriteTimeUtc(filePath):O}.");
}
}
Expand Down