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

PhysicalFileProvider and PhysicalFilesWatcher should implement IDisposable correctly #41918

Merged
merged 2 commits into from
Sep 11, 2020
Merged

PhysicalFileProvider and PhysicalFilesWatcher should implement IDisposable correctly #41918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2afb9f8
Changes in implementation of the IDisposable pattern for PhysicalFile…
Marusyk Sep 5, 2020
2945fe0
Remove the finalizers
Marusyk Sep 9, 2020
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
21 changes: 14 additions & 7 deletions src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFileProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public class PhysicalFileProvider : IFileProvider, IDisposable

private bool? _usePollingFileWatcher;
private bool? _useActivePolling;
private bool _disposed;

/// <summary>
/// Initializes a new instance of a PhysicalFileProvider at the given root directory.
Expand Down Expand Up @@ -177,22 +178,28 @@ private void ReadPollingEnvironmentVariables()
/// <summary>
/// Disposes the provider. Change tokens may not trigger after the provider is disposed.
/// </summary>
public void Dispose() => Dispose(true);
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
Copy link
Member

Choose a reason for hiding this comment

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

GC.SuppressFinalize(this); is not necessary on types that don't have finalizers. You removed the finalizer below.

Copy link
Member

Choose a reason for hiding this comment

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

This was discussed in #41918 (comment)

}

/// <summary>
/// Disposes the provider.
/// </summary>
/// <param name="disposing"><c>true</c> is invoked from <see cref="IDisposable.Dispose"/>.</param>
protected virtual void Dispose(bool disposing)
{
_fileWatcher?.Dispose();
if (!_disposed)
{
if (disposing)
{
_fileWatcher?.Dispose();
}
_disposed = true;
}
}

/// <summary>
/// Destructor for <see cref="PhysicalFileProvider"/>.
/// </summary>
~PhysicalFileProvider() => Dispose(false);

/// <summary>
/// The root directory for this instance.
/// </summary>
Expand Down
23 changes: 15 additions & 8 deletions src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFilesWatcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public class PhysicalFilesWatcher : IDisposable
private bool _timerInitialzed;
private object _timerLock = new object();
private Func<Timer> _timerFactory;
private bool _disposed;

/// <summary>
/// Initializes an instance of <see cref="PhysicalFilesWatcher" /> that watches files in <paramref name="root" />.
Expand Down Expand Up @@ -231,23 +232,29 @@ internal IChangeToken GetOrAddWildcardChangeToken(string pattern)
/// <summary>
/// Disposes the provider. Change tokens may not trigger after the provider is disposed.
/// </summary>
public void Dispose() => Dispose(true);
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}

/// <summary>
/// Disposes the provider.
/// </summary>
/// <param name="disposing"><c>true</c> is invoked from <see cref="IDisposable.Dispose"/>.</param>
protected virtual void Dispose(bool disposing)
{
_fileWatcher.Dispose();
_timer?.Dispose();
if (!_disposed)
{
if (disposing)
{
_fileWatcher?.Dispose();
_timer?.Dispose();
}
_disposed = true;
}
}

/// <summary>
/// Destructor for <see cref="PhysicalFilesWatcher"/>.
/// </summary>
~PhysicalFilesWatcher() => Dispose(false);

private void OnRenamed(object sender, RenamedEventArgs e)
{
// For a file name change or a directory's name change notify registered tokens.
Expand Down