Skip to content

Commit 68743c0

Browse files
ChrisPulmanChrisPulman
committed
Fix for Memory Leak
#659
1 parent 6bd466f commit 68743c0

File tree

7 files changed

+75
-83
lines changed

7 files changed

+75
-83
lines changed

‎samples/Directory.build.props

-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
<PropertyGroup>
33
<Nullable>enable</Nullable>
44
<AvaloniaVersion>11.0.10</AvaloniaVersion>
5-
<CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)..\src\analyzers.ruleset</CodeAnalysisRuleSet>
65
<WarningsAsErrors>CS8600;CS8602;CS8603;CS8604;CS8605;CS8606;CS8607;CS8608;CS8609;CS8610;CS8611;CS8612;CS8613;CS8614;CS8615;CS8616;CS8617;CS8618;CS8619;CS8620;CS8621;CS8622;CS8623;CS8624;CS8625</WarningsAsErrors>
76
</PropertyGroup>
87
<ItemGroup>

‎src/Directory.build.props

+49-45
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,52 @@
1-
<Project>
2-
<PropertyGroup>
3-
<Copyright>Copyright (c) .NET Foundation and Contributors</Copyright>
4-
<Product>ReactiveUI.Validations ($(TargetFramework))</Product>
5-
<PackageLicenseExpression>MIT</PackageLicenseExpression>
6-
<PackageProjectUrl>https://github.com/reactiveui/ReactiveUI.Validation/</PackageProjectUrl>
7-
<PackageIconUrl>https://github.com/reactiveui/ReactiveUI.Validation/blob/master/media/logo.png?raw=true</PackageIconUrl>
8-
<Authors>.NET Foundation and Contributors</Authors>
9-
<Description>Validations library for ReactiveUI.</Description>
10-
<Owners>xpaulbettsx;ghuntley</Owners>
11-
<PackageTags>reactiveui;validation library;reactive programming;xamarin forms;netcore;portable;xamarin;xamarin ios;xamarin mac;android;monodroid;uwp;net45</PackageTags>
12-
<PackageReleaseNotes>https://github.com/reactiveui/reactiveui.validation/releases</PackageReleaseNotes>
13-
<RepositoryUrl>https://github.com/reactiveui/reactiveui.validation</RepositoryUrl>
14-
<RepositoryType>git</RepositoryType>
15-
<GenerateDocumentationFile>true</GenerateDocumentationFile>
16-
<IsTestProject>$(MSBuildProjectName.Contains('Tests'))</IsTestProject>
17-
<DebugType>Embedded</DebugType>
18-
<!-- Optional: Publish the repository URL in the built .nupkg (in the NuSpec <Repository> element) -->
19-
<PublishRepositoryUrl>true</PublishRepositoryUrl>
20-
<!-- Optional: Embed source files that are not tracked by the source control manager in the PDB -->
21-
<EmbedUntrackedSources>true</EmbedUntrackedSources>
22-
<!-- Optional: Include PDB in the built .nupkg -->
23-
<AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb</AllowedOutputExtensionsInPackageBuildOutputFolder>
24-
<CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)analyzers.ruleset</CodeAnalysisRuleSet>
25-
<WarningsAsErrors>CS8600;CS8602;CS8603;CS8604;CS8605;CS8606;CS8607;CS8608;CS8609;CS8610;CS8611;CS8612;CS8613;CS8614;CS8615;CS8616;CS8617;CS8618;CS8619;CS8620;CS8621;CS8622;CS8623;CS8624;CS8625</WarningsAsErrors>
26-
<LangVersion>preview</LangVersion>
27-
<EnableNETAnalyzers>True</EnableNETAnalyzers>
28-
<AnalysisLevel>latest</AnalysisLevel>
29-
</PropertyGroup>
1+
<Project>
2+
<PropertyGroup>
3+
<Copyright>Copyright (c) .NET Foundation and Contributors</Copyright>
4+
<Product>ReactiveUI.Validations ($(TargetFramework))</Product>
5+
<PackageLicenseExpression>MIT</PackageLicenseExpression>
6+
<PackageProjectUrl>https://github.com/reactiveui/ReactiveUI.Validation/</PackageProjectUrl>
7+
<PackageIconUrl>https://github.com/reactiveui/ReactiveUI.Validation/blob/master/media/logo.png?raw=true</PackageIconUrl>
8+
<Authors>.NET Foundation and Contributors</Authors>
9+
<Description>Validations library for ReactiveUI.</Description>
10+
<Owners>xanaisbettsx;ghuntley</Owners>
11+
<PackageTags>reactiveui;validation library;reactive programming;netcore;netstandard;netframework</PackageTags>
12+
<PackageIcon>logo.png</PackageIcon>
13+
<PackageReadmeFile>README.md</PackageReadmeFile>
14+
<PackageReleaseNotes>https://github.com/reactiveui/reactiveui.validation/releases</PackageReleaseNotes>
15+
<RepositoryUrl>https://github.com/reactiveui/reactiveui.validation</RepositoryUrl>
16+
<RepositoryType>git</RepositoryType>
17+
<GenerateDocumentationFile>true</GenerateDocumentationFile>
18+
<IsTestProject>$(MSBuildProjectName.Contains('Tests'))</IsTestProject>
19+
<DebugType>Embedded</DebugType>
20+
<!-- Optional: Publish the repository URL in the built .nupkg (in the NuSpec <Repository> element) -->
21+
<PublishRepositoryUrl>true</PublishRepositoryUrl>
22+
<!-- Optional: Embed source files that are not tracked by the source control manager in the PDB -->
23+
<EmbedUntrackedSources>true</EmbedUntrackedSources>
24+
<!-- Optional: Include PDB in the built .nupkg -->
25+
<AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb</AllowedOutputExtensionsInPackageBuildOutputFolder>
26+
<WarningsAsErrors>CS8600;CS8602;CS8603;CS8604;CS8605;CS8606;CS8607;CS8608;CS8609;CS8610;CS8611;CS8612;CS8613;CS8614;CS8615;CS8616;CS8617;CS8618;CS8619;CS8620;CS8621;CS8622;CS8623;CS8624;CS8625</WarningsAsErrors>
27+
<LangVersion>preview</LangVersion>
28+
<EnableNETAnalyzers>True</EnableNETAnalyzers>
29+
<AnalysisLevel>latest</AnalysisLevel>
30+
<NoWarn>$(NoWarn);IDE1006</NoWarn>
31+
</PropertyGroup>
3032

31-
<ItemGroup>
32-
<None Include="$(MSBuildThisFileDirectory)..\LICENSE" Pack="true" PackagePath="LICENSE" />
33-
</ItemGroup>
33+
<ItemGroup>
34+
<None Include="$(MSBuildThisFileDirectory)..\media\logo.png" Pack="true" PackagePath="\" />
35+
<None Include="$(MSBuildThisFileDirectory)..\LICENSE" Pack="true" PackagePath="LICENSE" />
36+
<None Include="$(MSBuildThisFileDirectory)..\README.md" Pack="true" PackagePath="\" />
37+
</ItemGroup>
3438

35-
<ItemGroup Condition="'$(IsTestProject)' != 'true'">
36-
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="All" />
37-
</ItemGroup>
38-
<ItemGroup>
39-
<PackageReference Include="Nerdbank.GitVersioning" Version="3.6.133" PrivateAssets="all" />
40-
<PackageReference Include="stylecop.analyzers" Version="1.2.0-beta.556" PrivateAssets="all" />
41-
<PackageReference Include="Roslynator.Analyzers" Version="4.12.2" PrivateAssets="All" />
42-
</ItemGroup>
43-
<ItemGroup>
44-
<AdditionalFiles Include="$(MSBuildThisFileDirectory)stylecop.json" Link="stylecop.json" />
45-
</ItemGroup>
39+
<ItemGroup Condition="'$(IsTestProject)' != 'true'">
40+
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="All" />
41+
</ItemGroup>
42+
<ItemGroup>
43+
<PackageReference Include="Nerdbank.GitVersioning" Version="3.6.133" PrivateAssets="all" />
44+
<PackageReference Include="stylecop.analyzers" Version="1.2.0-beta.556" PrivateAssets="all" />
45+
<PackageReference Include="Roslynator.Analyzers" Version="4.12.2" PrivateAssets="All" />
46+
</ItemGroup>
47+
<ItemGroup>
48+
<AdditionalFiles Include="$(MSBuildThisFileDirectory)stylecop.json" Link="stylecop.json" />
49+
</ItemGroup>
50+
51+
</Project>
4652

47-
</Project>
48-

‎src/ReactiveUI.Validation.Tests/MemoryLeakTests.cs

+5-13
Original file line numberDiff line numberDiff line change
@@ -37,28 +37,20 @@ public void Instance_Released_IsGarbageCollected()
3737
new Action(
3838
() =>
3939
{
40-
var sut = new TestClassMemory();
40+
var memTest = new TestClassMemory();
4141

42-
reference = new WeakReference(sut, true);
43-
sut.Dispose();
42+
reference = new WeakReference(memTest, true);
43+
memTest.Dispose();
4444
})();
4545

46-
// Sut should have gone out of scope about now, so the garbage collector can clean it up
46+
// memTest should have gone out of scope about now, so the garbage collector can clean it up
4747
dotMemory.Check(
4848
memory => memory.GetObjects(
4949
where => where.Type.Is<TestClassMemory>()).ObjectsCount.Should().Be(0, "it is out of scope"));
5050

5151
GC.Collect();
5252
GC.WaitForPendingFinalizers();
5353

54-
if (reference.Target is TestClassMemory sut)
55-
{
56-
// ReactiveObject does not inherit from IDisposable, so we need to check ValidationContext
57-
sut.ValidationContext.Should().BeNull("it is garbage collected");
58-
}
59-
else
60-
{
61-
reference.Target.Should().BeNull("it is garbage collected");
62-
}
54+
reference.Target.Should().BeNull("it is garbage collected");
6355
}
6456
}

‎src/ReactiveUI.Validation.Tests/Models/TestClassMemory.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public TestClassMemory()
3131
.DisposeWith(_disposable);
3232

3333
// commenting out the following statement makes the test green
34-
ValidationContext.ValidationStatusChange
34+
ValidationContext?.ValidationStatusChange
3535
.Subscribe(/* you do something here, but this does not matter for now. */)
3636
.DisposeWith(_disposable);
3737
}

‎src/ReactiveUI.Validation.Tests/ValidationContextTests.cs

+3-2
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,15 @@ public void IsValidShouldNotifyOfValidityChange()
125125
viewModel.ValidationContext.Add(nameValidation);
126126

127127
var latestValidity = false;
128-
viewModel.IsValid().Subscribe(isValid => latestValidity = isValid);
128+
var d = viewModel.IsValid().Subscribe(isValid => latestValidity = isValid);
129129
Assert.False(latestValidity);
130130

131131
viewModel.Name = "Jonathan";
132132
Assert.True(latestValidity);
133133

134134
viewModel.Name = string.Empty;
135135
Assert.False(latestValidity);
136+
d.Dispose();
136137
}
137138

138139
/// <summary>
@@ -220,4 +221,4 @@ public void ShouldClearAttachedValidationRulesForTheGivenProperty()
220221
Assert.NotEmpty(viewModel.ValidationContext.Text);
221222
Assert.Equal(name2ErrorMessage, viewModel.ValidationContext.Text.ToSingleLine());
222223
}
223-
}
224+
}

‎src/ReactiveUI.Validation/Contexts/ValidationContext.cs

+12-13
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using System;
77
using System.Buffers;
88
using System.Collections.Generic;
9-
using System.Diagnostics.CodeAnalysis;
109
using System.Linq;
1110
using System.Reactive.Concurrency;
1211
using System.Reactive.Disposables;
@@ -31,15 +30,17 @@ namespace ReactiveUI.Validation.Contexts;
3130
/// </remarks>
3231
public class ValidationContext : ReactiveObject, IValidationContext
3332
{
33+
private static readonly CompositeDisposable _collectionDisposables = [];
34+
private readonly CompositeDisposable _disposables = [];
35+
3436
private readonly ReplaySubject<IValidationState> _validationStatusChange = new(1);
3537
private readonly ReplaySubject<bool> _validSubject = new(1);
3638

37-
private readonly IConnectableObservable<bool> _validationConnectable;
39+
private readonly IObservable<bool> _validationObservable;
3840
private readonly ObservableAsPropertyHelper<IValidationText> _validationText;
3941
private readonly ObservableAsPropertyHelper<bool> _isValid;
4042

41-
private readonly CompositeDisposable _disposables = [];
42-
private SourceList<IValidationComponent> _validationSource = new();
43+
private readonly SourceList<IValidationComponent> _validationSource = new();
4344
private bool _isActive;
4445

4546
/// <summary>
@@ -52,15 +53,15 @@ public ValidationContext(IScheduler? scheduler = null)
5253
var changeSets = _validationSource.Connect().ObserveOn(scheduler);
5354
Validations = changeSets.AsObservableList();
5455

55-
_validationConnectable = changeSets
56+
_validationObservable = changeSets
5657
.StartWithEmpty()
5758
.AutoRefreshOnObservable(x => x.ValidationStatusChange)
5859
.QueryWhenChanged(static x =>
5960
{
6061
using ReadOnlyCollectionPooled<IValidationComponent> validationComponents = new(x);
62+
validationComponents.DisposeWith(_collectionDisposables);
6163
return validationComponents.Count is 0 || validationComponents.All(v => v.IsValid);
62-
})
63-
.Multicast(_validSubject);
64+
});
6465

6566
_isValid = _validSubject
6667
.StartWith(true)
@@ -95,7 +96,7 @@ public IObservable<bool> Valid
9596
/// <summary>
9697
/// Gets get the list of validations.
9798
/// </summary>
98-
public IObservableList<IValidationComponent> Validations { get; private set; }
99+
public IObservableList<IValidationComponent> Validations { get; }
99100

100101
/// <inheritdoc/>
101102
public bool IsValid
@@ -183,11 +184,9 @@ protected virtual void Dispose(bool disposing)
183184
_validationStatusChange.Dispose();
184185
_validSubject.Dispose();
185186
_validationSource.Clear();
186-
Validations.Dispose();
187187
_validationSource.Dispose();
188-
189-
Validations = null!;
190-
_validationSource = null!;
188+
Validations.Dispose();
189+
_collectionDisposables.Dispose();
191190
}
192191
}
193192

@@ -199,7 +198,7 @@ private void Activate()
199198
}
200199

201200
_isActive = true;
202-
_disposables.Add(_validationConnectable.Connect());
201+
_disposables.Add(_validationObservable.Subscribe(_validSubject));
203202
}
204203

205204
/// <summary>

‎src/ReactiveUI.Validation/Helpers/ReactiveValidationObject.cs

+5-8
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ namespace ReactiveUI.Validation.Helpers;
2727
/// </summary>
2828
public abstract class ReactiveValidationObject : ReactiveObject, IValidatableViewModel, INotifyDataErrorInfo, IDisposable
2929
{
30-
private CompositeDisposable _disposables = [];
31-
private IValidationTextFormatter<string> _formatter;
32-
private HashSet<string> _mentionedPropertyNames = [];
30+
private readonly CompositeDisposable _disposables = [];
31+
private readonly IValidationTextFormatter<string> _formatter;
32+
private readonly HashSet<string> _mentionedPropertyNames = [];
3333
private bool _hasErrors;
3434

3535
/// <summary>
@@ -77,7 +77,7 @@ public bool HasErrors
7777
}
7878

7979
/// <inheritdoc />
80-
public IValidationContext ValidationContext { get; private set; }
80+
public IValidationContext ValidationContext { get; }
8181

8282
/// <summary>
8383
/// Returns a collection of error messages, required by the INotifyDataErrorInfo interface.
@@ -121,11 +121,8 @@ protected virtual void Dispose(bool disposing)
121121
if (!_disposables.IsDisposed && disposing)
122122
{
123123
_disposables.Dispose();
124+
ValidationContext.Dispose();
124125
_mentionedPropertyNames.Clear();
125-
_formatter = null!;
126-
_mentionedPropertyNames = null!;
127-
_disposables = null!;
128-
ValidationContext = null!;
129126
}
130127
}
131128

0 commit comments

Comments
 (0)