Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 776aae5

Browse files
authored
Don't root FileSystemWatcher unnecessarily (#41872)
We already try to do this on Unix, though it seems we don't currently have tests for it, but we don't on Windows.
1 parent 4746f75 commit 776aae5

File tree

2 files changed

+50
-20
lines changed

2 files changed

+50
-20
lines changed

src/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Win32.cs

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,19 @@ private void StartRaisingEvents()
5151
// Store all state, including a preallocated overlapped, into the state object that'll be
5252
// passed from iteration to iteration during the lifetime of the operation. The buffer will be pinned
5353
// from now until the end of the operation.
54-
state = new AsyncReadState(session, buffer, _directoryHandle, ThreadPoolBoundHandle.BindHandle(_directoryHandle));
55-
unsafe { state.PreAllocatedOverlapped = new PreAllocatedOverlapped(ReadDirectoryChangesCallback, state, buffer); }
54+
state = new AsyncReadState(session, buffer, _directoryHandle, ThreadPoolBoundHandle.BindHandle(_directoryHandle), this);
55+
unsafe
56+
{
57+
state.PreAllocatedOverlapped = new PreAllocatedOverlapped((errorCode, numBytes, overlappedPointer) =>
58+
{
59+
AsyncReadState state = (AsyncReadState)ThreadPoolBoundHandle.GetNativeOverlappedState(overlappedPointer);
60+
state.ThreadPoolBinding.FreeNativeOverlapped(overlappedPointer);
61+
if (state.WeakWatcher.TryGetTarget(out FileSystemWatcher watcher))
62+
{
63+
watcher.ReadDirectoryChangesCallback(errorCode, numBytes, state);
64+
}
65+
}, state, buffer);
66+
}
5667
}
5768
catch
5869
{
@@ -191,9 +202,8 @@ private unsafe void Monitor(AsyncReadState state)
191202
}
192203

193204
/// <summary>Callback invoked when an asynchronous read on the directory handle completes.</summary>
194-
private unsafe void ReadDirectoryChangesCallback(uint errorCode, uint numBytes, NativeOverlapped* overlappedPointer)
205+
private void ReadDirectoryChangesCallback(uint errorCode, uint numBytes, AsyncReadState state)
195206
{
196-
AsyncReadState state = (AsyncReadState)ThreadPoolBoundHandle.GetNativeOverlappedState(overlappedPointer);
197207
try
198208
{
199209
if (IsHandleInvalid(state.DirectoryHandle))
@@ -232,11 +242,7 @@ private unsafe void ReadDirectoryChangesCallback(uint errorCode, uint numBytes,
232242
}
233243
finally
234244
{
235-
// Clean up state associated with this one iteration
236-
state.ThreadPoolBinding.FreeNativeOverlapped(overlappedPointer);
237-
238-
// Then call Monitor again to either start the next iteration or
239-
// clean up the whole operation.
245+
// Call Monitor again to either start the next iteration or clean up the whole operation.
240246
Monitor(state);
241247
}
242248
}
@@ -338,7 +344,7 @@ private unsafe void ParseEventBufferAndNotifyForEach(byte[] buffer)
338344
/// </summary>
339345
private sealed class AsyncReadState
340346
{
341-
internal AsyncReadState(int session, byte[] buffer, SafeFileHandle handle, ThreadPoolBoundHandle binding)
347+
internal AsyncReadState(int session, byte[] buffer, SafeFileHandle handle, ThreadPoolBoundHandle binding, FileSystemWatcher parent)
342348
{
343349
Debug.Assert(buffer != null);
344350
Debug.Assert(buffer.Length > 0);
@@ -349,13 +355,15 @@ internal AsyncReadState(int session, byte[] buffer, SafeFileHandle handle, Threa
349355
Buffer = buffer;
350356
DirectoryHandle = handle;
351357
ThreadPoolBinding = binding;
358+
WeakWatcher = new WeakReference<FileSystemWatcher>(parent);
352359
}
353360

354-
internal int Session { get; private set; }
355-
internal byte[] Buffer { get; private set; }
356-
internal SafeFileHandle DirectoryHandle { get; private set; }
357-
internal ThreadPoolBoundHandle ThreadPoolBinding { get; private set; }
358-
internal PreAllocatedOverlapped PreAllocatedOverlapped { get; set; }
361+
internal int Session { get; }
362+
internal byte[] Buffer { get; }
363+
internal SafeFileHandle DirectoryHandle { get; }
364+
internal ThreadPoolBoundHandle ThreadPoolBinding { get; }
365+
internal PreAllocatedOverlapped PreAllocatedOverlapped { get; set; }
366+
internal WeakReference<FileSystemWatcher> WeakWatcher { get; }
359367
}
360368
}
361369
}

src/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.cs

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,8 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5-
using System;
6-
using System.Collections.Generic;
75
using System.ComponentModel;
8-
using System.IO;
9-
using System.Linq;
10-
using System.Runtime.InteropServices;
6+
using System.Runtime.CompilerServices;
117
using System.Threading;
128
using Xunit;
139

@@ -265,5 +261,31 @@ public void EndInit_DoesNotEnableEventRaisedEvents()
265261
Thread.Sleep(WaitForExpectedEventTimeout);
266262
}
267263
}
264+
265+
[Fact]
266+
public void DroppedWatcher_Collectible()
267+
{
268+
WeakReference watcher = CreateEnabledWatcher(TestDirectory);
269+
File.Create(GetTestFilePath()).Dispose();
270+
Assert.True(SpinWait.SpinUntil(() =>
271+
{
272+
GC.Collect();
273+
return !watcher.IsAlive;
274+
}, LongWaitTimeout));
275+
276+
}
277+
278+
[MethodImpl(MethodImplOptions.NoInlining)]
279+
private static WeakReference CreateEnabledWatcher(string path)
280+
{
281+
var fsw = new FileSystemWatcher(path);
282+
fsw.Created += (s, e) => { };
283+
fsw.Deleted += (s, e) => { };
284+
fsw.Changed += (s, e) => { };
285+
fsw.Renamed += (s, e) => { };
286+
fsw.Error += (s, e) => { };
287+
fsw.EnableRaisingEvents = true;
288+
return new WeakReference(fsw);
289+
}
268290
}
269291
}

0 commit comments

Comments
 (0)