Skip to content

Commit 642b77c

Browse files
xoofxxoofx
authored
Merge pull request #34 from Rohansi/fix-disposed-watcher-removal
Fix dispose not removing the watchers from aggregate and mount FS
2 parents a8c4ff7 + 2ca66ad commit 642b77c

12 files changed

+170
-56
lines changed

‎src/Zio.Tests/AssertEx.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using Xunit;
5+
6+
namespace Zio.Tests
7+
{
8+
internal static class AssertEx
9+
{
10+
public static void Equivalent<T>(IEnumerable<T> expected, IEnumerable<T> actual)
11+
where T : IComparable<T>
12+
{
13+
Assert.Equal(expected.OrderBy(i => i), actual.OrderBy(i => i));
14+
}
15+
}
16+
}

‎src/Zio.Tests/FileSystems/TestAggregateFileSystem.cs‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
// See the license.txt file in the project root for more information.
44

55
using System;
6-
using System.Linq;
6+
using System.Collections;
7+
using System.Reflection;
78
using Xunit;
89
using Zio.FileSystems;
910

@@ -51,6 +52,27 @@ public void TestWatcher()
5152
Assert.True(gotChange2);
5253
}
5354

55+
[Fact]
56+
public void TestWatcherRemovedWhenDisposed()
57+
{
58+
var fs = GetCommonAggregateFileSystem();
59+
60+
var watcher = fs.Watch("/");
61+
watcher.IncludeSubdirectories = true;
62+
watcher.EnableRaisingEvents = true;
63+
watcher.Dispose();
64+
65+
System.Threading.Thread.Sleep(100);
66+
67+
var watchersField = typeof(AggregateFileSystem).GetTypeInfo()
68+
.GetField("_watchers", BindingFlags.NonPublic | BindingFlags.Instance);
69+
Assert.NotNull(watchersField);
70+
71+
var watchers = (IList)watchersField.GetValue(fs);
72+
Assert.NotNull(watchers);
73+
Assert.Empty(watchers);
74+
}
75+
5476
[Fact]
5577
public void TestAddRemoveFileSystem()
5678
{
@@ -69,18 +91,18 @@ public void TestAddRemoveFileSystem()
6991
Assert.Throws<ArgumentException>(() => fs.RemoveFileSystem(memfs2));
7092

7193
var list = fs.GetFileSystems();
72-
Assert.Equal(1, list.Count);
94+
Assert.Single(list);
7395
Assert.Equal(memfs, list[0]);
7496

7597
fs.ClearFileSystems();
76-
Assert.Equal(0, fs.GetFileSystems().Count);
98+
Assert.Empty(fs.GetFileSystems());
7799

78100
fs.SetFileSystems(list);
79101

80102
fs.RemoveFileSystem(memfs);
81103

82104
list = fs.GetFileSystems();
83-
Assert.Equal(0, list.Count);
105+
Assert.Empty(list);
84106
}
85107

86108
[Fact]
@@ -112,7 +134,7 @@ public void TestFindFileSystemEntries()
112134

113135
{
114136
var entries = fs.FindFileSystemEntries("/b");
115-
Assert.Equal(1, entries.Count);
137+
Assert.Single(entries);
116138

117139
Assert.IsType<DirectoryEntry>(entries[0]);
118140
Assert.Equal(memfs2, entries[0].FileSystem);

‎src/Zio.Tests/FileSystems/TestFileSystemBase.cs‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -317,19 +317,19 @@ class EnumeratePathsResult
317317

318318
public void Check(EnumeratePathsResult other)
319319
{
320-
Assert.Equal(TopDirs, other.TopDirs);
321-
Assert.Equal(TopFiles, other.TopFiles);
322-
Assert.Equal(TopEntries, other.TopEntries);
323-
324-
Assert.Equal(AllDirs, other.AllDirs);
325-
Assert.Equal(AllFiles, other.AllFiles);
326-
Assert.Equal(AllEntries, other.AllEntries);
327-
328-
Assert.Equal(AllFiles_txt, other.AllFiles_txt);
329-
Assert.Equal(AllFiles_i, other.AllFiles_i);
330-
Assert.Equal(AllEntries_b, other.AllEntries_b);
331-
Assert.Equal(AllDirs_a1, other.AllDirs_a1);
332-
Assert.Equal(AllDirs_a2, other.AllDirs_a2);
320+
AssertEx.Equivalent(TopDirs, other.TopDirs);
321+
AssertEx.Equivalent(TopFiles, other.TopFiles);
322+
AssertEx.Equivalent(TopEntries, other.TopEntries);
323+
324+
AssertEx.Equivalent(AllDirs, other.AllDirs);
325+
AssertEx.Equivalent(AllFiles, other.AllFiles);
326+
AssertEx.Equivalent(AllEntries, other.AllEntries);
327+
328+
AssertEx.Equivalent(AllFiles_txt, other.AllFiles_txt);
329+
AssertEx.Equivalent(AllFiles_i, other.AllFiles_i);
330+
AssertEx.Equivalent(AllEntries_b, other.AllEntries_b);
331+
AssertEx.Equivalent(AllDirs_a1, other.AllDirs_a1);
332+
AssertEx.Equivalent(AllDirs_a2, other.AllDirs_a2);
333333
}
334334

335335
public EnumeratePathsResult(IFileSystem fs)

‎src/Zio.Tests/FileSystems/TestFileSystemCompactBase.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public void TestFile()
162162
Assert.Equal(new List<string>() {"/tata.txt", "/titi.txt"}, files);
163163

164164
var dirs = fs.EnumerateDirectories("/").Select(p => p.FullName).ToList();
165-
Assert.Equal(0, dirs.Count);
165+
Assert.Empty(dirs);
166166

167167
// Check ReplaceFile
168168
var originalContent2 = "this is a content2";
@@ -342,7 +342,7 @@ public void TestDirectoryDeleteAndOpenFile()
342342
fs.DeleteDirectory("/dir", true);
343343

344344
var entries = fs.EnumeratePaths("/").ToList();
345-
Assert.Equal(0, entries.Count);
345+
Assert.Empty(entries);
346346
}
347347

348348
[Fact]

‎src/Zio.Tests/FileSystems/TestFileSystemEntries.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,10 @@ public void TestFileEntry()
9292
var subFolder = mydir.CreateSubdirectory("sub");
9393
Assert.True(subFolder.Exists);
9494

95-
Assert.Equal(0, mydir.EnumerateFiles().ToList().Count);
95+
Assert.Empty(mydir.EnumerateFiles());
9696

9797
var subDirs = mydir.EnumerateDirectories().ToList();
98-
Assert.Equal(1, subDirs.Count);
98+
Assert.Single(subDirs);
9999
Assert.Equal("/yoyo/sub", subDirs[0].FullName);
100100

101101
mydir.Delete();
@@ -117,11 +117,11 @@ public void TestFileEntry()
117117
Assert.Equal("abcdefghi", file.ReadAllText());
118118

119119
var lines = file.ReadAllLines();
120-
Assert.Equal(1, lines.Length);
120+
Assert.Single(lines);
121121
Assert.Equal("abcdefghi", lines[0]);
122122

123123
lines = file.ReadAllLines(Encoding.UTF8);
124-
Assert.Equal(1, lines.Length);
124+
Assert.Single(lines);
125125
Assert.Equal("abcdefghi", lines[0]);
126126

127127
Assert.Equal(new byte[] { 1, 2, 3 }, yoyo.ReadAllBytes());

‎src/Zio.Tests/FileSystems/TestMountFileSystem.cs‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
// See the license.txt file in the project root for more information.
44

55
using System;
6+
using System.Collections;
67
using System.Collections.Generic;
78
using System.IO;
89
using System.Linq;
10+
using System.Reflection;
911
using Xunit;
1012
using Zio.FileSystems;
1113

@@ -136,7 +138,7 @@ public void TestMount()
136138

137139
fs.Unmount("/test2");
138140

139-
Assert.Equal(0, fs.GetMounts().Count);
141+
Assert.Empty(fs.GetMounts());
140142

141143
var innerFs = GetCommonMemoryFileSystem();
142144
fs.Mount("/x/y", innerFs);
@@ -145,6 +147,27 @@ public void TestMount()
145147
Assert.True(fs.FileExists("/x/y/b/A.txt"));
146148
}
147149

150+
[Fact]
151+
public void TestWatcherRemovedWhenDisposed()
152+
{
153+
var fs = GetCommonMountFileSystemWithMounts();
154+
155+
var watcher = fs.Watch("/");
156+
watcher.IncludeSubdirectories = true;
157+
watcher.EnableRaisingEvents = true;
158+
watcher.Dispose();
159+
160+
System.Threading.Thread.Sleep(100);
161+
162+
var watchersField = typeof(MountFileSystem).GetTypeInfo()
163+
.GetField("_watchers", BindingFlags.NonPublic | BindingFlags.Instance);
164+
Assert.NotNull(watchersField);
165+
166+
var watchers = (IList)watchersField.GetValue(fs);
167+
Assert.NotNull(watchers);
168+
Assert.Empty(watchers);
169+
}
170+
148171
[Fact]
149172
public void EnumerateDeepMount()
150173
{

‎src/Zio.Tests/TestSearchPattern.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ public class TestSearchPattern
2222

2323
// Test exact match
2424
[InlineData("/a/b/c", "x", "x", true)]
25-
[InlineData("/a/b/c", "d/x", "x", true)]
2625
[InlineData("/a/b/c", "x", "d/x", true)]
2726
[InlineData("/a/b/c", "x", "d/e/x", true)]
2827

@@ -53,7 +52,6 @@ public class TestSearchPattern
5352
[InlineData("/a/b/c", "*.i", "x.i", true)]
5453
[InlineData("/a/b/c", "*.i", "x.i1", false)]
5554
[InlineData("/a/b/c", "x*.txt", "x_txt", false)]
56-
[InlineData("/a/b/c", "x?z", "d/xyz", true)]
5755
public void TestMatch(string path, string searchPattern, string pathToSearch, bool match = true)
5856
{
5957
var pathInfo = new UPath(path);

‎src/Zio.Tests/TestUPath.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@ public void TestAbsoluteAndRelative()
103103
[InlineData("\\", "\\", "/")]
104104
[InlineData("//", "//", "/")]
105105
[InlineData("", "/", "/")]
106-
[InlineData("", "", "")]
107106
[InlineData("a", "b", "a/b")]
108107
[InlineData("a/b", "c", "a/b/c")]
109108
[InlineData("", "b", "b")]

‎src/Zio.Tests/Zio.Tests.csproj‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
4-
<TargetFramework>netcoreapp1.1</TargetFramework>
4+
<TargetFramework>netcoreapp2.1</TargetFramework>
5+
<RuntimeFrameworkVersion>2.1.6</RuntimeFrameworkVersion>
56
<DebugType>Full</DebugType>
67
</PropertyGroup>
78

89
<ItemGroup>
9-
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.0.0" />
10-
<PackageReference Include="xunit" Version="2.3.0-beta2-build3683" />
11-
<PackageReference Include="xunit.runner.visualstudio" Version="2.3.0-beta2-build1317" />
12-
<DotNetCliToolReference Include="dotnet-xunit" Version="2.3.0-beta2-build3683" />
10+
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.9.0" />
11+
<PackageReference Include="xunit" Version="2.3.1" />
12+
<PackageReference Include="xunit.runner.visualstudio" Version="2.3.1">
13+
<PrivateAssets>all</PrivateAssets>
14+
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
15+
</PackageReference>
16+
<DotNetCliToolReference Include="dotnet-xunit" Version="2.3.1" />
1317
</ItemGroup>
1418

1519
<ItemGroup>

‎src/Zio/FileSystems/AggregateFileSystem.cs‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ namespace Zio.FileSystems
1515
public class AggregateFileSystem : ReadOnlyFileSystem
1616
{
1717
private readonly List<IFileSystem> _fileSystems;
18-
private readonly List<AggregateFileSystemWatcher> _watchers;
18+
private readonly List<Watcher> _watchers;
1919

2020
/// <summary>
2121
/// Initializes a new instance of the <see cref="AggregateFileSystem"/> class.
@@ -34,7 +34,7 @@ public AggregateFileSystem(bool owned = true) : this(null, owned)
3434
public AggregateFileSystem(IFileSystem fileSystem, bool owned = true) : base(fileSystem, owned)
3535
{
3636
_fileSystems = new List<IFileSystem>();
37-
_watchers = new List<AggregateFileSystemWatcher>();
37+
_watchers = new List<Watcher>();
3838
}
3939

4040
protected override void Dispose(bool disposing)
@@ -57,10 +57,7 @@ protected override void Dispose(bool disposing)
5757
}
5858

5959
_fileSystems.Clear();
60-
}
6160

62-
lock (_watchers)
63-
{
6461
foreach (var watcher in _watchers)
6562
{
6663
watcher.Dispose();
@@ -418,7 +415,7 @@ protected override IFileSystemWatcher WatchImpl(UPath path)
418415
{
419416
lock (_fileSystems)
420417
{
421-
var watcher = new AggregateFileSystemWatcher(this, path);
418+
var watcher = new Watcher(this, path);
422419

423420
if (NextFileSystem != null && NextFileSystem.CanWatch(path))
424421
{
@@ -438,6 +435,30 @@ protected override IFileSystemWatcher WatchImpl(UPath path)
438435
}
439436
}
440437

438+
private class Watcher : AggregateFileSystemWatcher
439+
{
440+
private readonly AggregateFileSystem _fileSystem;
441+
442+
public Watcher(AggregateFileSystem fileSystem, UPath path)
443+
: base(fileSystem, path)
444+
{
445+
_fileSystem = fileSystem;
446+
}
447+
448+
protected override void Dispose(bool disposing)
449+
{
450+
base.Dispose(disposing);
451+
452+
if (disposing && !_fileSystem.IsDisposing)
453+
{
454+
lock (_fileSystem._fileSystems)
455+
{
456+
_fileSystem._watchers.Remove(this);
457+
}
458+
}
459+
}
460+
}
461+
441462
// ----------------------------------------------
442463
// Internals API
443464
// Used to retrieve the correct paths

0 commit comments

Comments
 (0)
⚡