Skip to content

Commit 3834d5e

Browse files
ChrisPulmanChrisPulman
authored
Fix for Memory Leak (#660)
* Fix for Memory Leak #659 * Update MemoryLeakTests.cs * Remove ReadOnlyCollectionPooled Use ReadOnlyDisposableCollection
1 parent efe8b22 commit 3834d5e

10 files changed

+102
-168
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

+13-13
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
<LangVersion>preview</LangVersion>
2828
<EnableNETAnalyzers>True</EnableNETAnalyzers>
2929
<AnalysisLevel>latest</AnalysisLevel>
30+
<NoWarn>$(NoWarn);IDE1006</NoWarn>
3031
</PropertyGroup>
3132

3233
<ItemGroup>
@@ -35,17 +36,16 @@
3536
<None Include="$(MSBuildThisFileDirectory)..\README.md" Pack="true" PackagePath="\" />
3637
</ItemGroup>
3738

38-
<ItemGroup Condition="'$(IsTestProject)' != 'true'">
39-
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="All" />
40-
</ItemGroup>
41-
<ItemGroup>
42-
<PackageReference Include="Nerdbank.GitVersioning" Version="3.6.133" PrivateAssets="all" />
43-
<PackageReference Include="stylecop.analyzers" Version="1.2.0-beta.556" PrivateAssets="all" />
44-
<PackageReference Include="Roslynator.Analyzers" Version="4.12.2" PrivateAssets="All" />
45-
</ItemGroup>
46-
<ItemGroup>
47-
<AdditionalFiles Include="$(MSBuildThisFileDirectory)stylecop.json" Link="stylecop.json" />
48-
</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>
4950

50-
</Project>
51-
51+
</Project>

‎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.IsAlive.Should().BeFalse("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/Collections/ReadOnlyCollectionPooled.cs

-112
This file was deleted.

‎src/ReactiveUI.Validation/Collections/ReadOnlyDisposableCollection{T}.cs

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Copyright (c) 2024 .NET Foundation and Contributors. All rights reserved.
2+
// Licensed to the .NET Foundation under one or more agreements.
3+
// The .NET Foundation licenses this file to you under the MIT license.
4+
// See the LICENSE file in the project root for full license information.
5+
6+
using System;
7+
using System.Collections;
8+
using System.Collections.Generic;
9+
using System.Collections.Immutable;
10+
11+
namespace ReactiveUI.Validation.Collections;
12+
13+
internal sealed class ReadOnlyDisposableCollection<T>(IEnumerable<T> items) : IReadOnlyCollection<T>, IDisposable
14+
{
15+
private readonly ImmutableList<T> _immutableList = ImmutableList.CreateRange(items);
16+
private bool _disposedValue;
17+
18+
/// <summary>
19+
/// Gets the number of elements in the collection.
20+
/// </summary>
21+
public int Count => _immutableList.Count;
22+
23+
/// <summary>
24+
/// Returns an enumerator that iterates through the collection.
25+
/// </summary>
26+
/// <returns>
27+
/// An enumerator that can be used to iterate through the collection.
28+
/// </returns>
29+
public IEnumerator<T> GetEnumerator() => _immutableList.GetEnumerator();
30+
31+
/// <summary>
32+
/// Returns an enumerator that iterates through a collection.
33+
/// </summary>
34+
/// <returns>
35+
/// An <see cref="T:System.Collections.IEnumerator"></see> object that can be used to iterate through the collection.
36+
/// </returns>
37+
IEnumerator IEnumerable.GetEnumerator() => _immutableList.GetEnumerator();
38+
39+
/// <summary>
40+
/// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
41+
/// </summary>
42+
public void Dispose()
43+
{
44+
Dispose(disposing: true);
45+
GC.SuppressFinalize(this);
46+
}
47+
48+
private void Dispose(bool disposing)
49+
{
50+
if (!_disposedValue)
51+
{
52+
if (disposing)
53+
{
54+
_immutableList.Clear();
55+
}
56+
57+
_disposedValue = true;
58+
}
59+
}
60+
}

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

+10-17
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,16 @@ namespace ReactiveUI.Validation.Contexts;
3130
/// </remarks>
3231
public class ValidationContext : ReactiveObject, IValidationContext
3332
{
33+
private readonly CompositeDisposable _disposables = [];
34+
3435
private readonly ReplaySubject<IValidationState> _validationStatusChange = new(1);
3536
private readonly ReplaySubject<bool> _validSubject = new(1);
3637

37-
private readonly IConnectableObservable<bool> _validationConnectable;
38+
private readonly IObservable<bool> _validationObservable;
3839
private readonly ObservableAsPropertyHelper<IValidationText> _validationText;
3940
private readonly ObservableAsPropertyHelper<bool> _isValid;
4041

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

4545
/// <summary>
@@ -52,15 +52,14 @@ public ValidationContext(IScheduler? scheduler = null)
5252
var changeSets = _validationSource.Connect().ObserveOn(scheduler);
5353
Validations = changeSets.AsObservableList();
5454

55-
_validationConnectable = changeSets
55+
_validationObservable = changeSets
5656
.StartWithEmpty()
5757
.AutoRefreshOnObservable(x => x.ValidationStatusChange)
5858
.QueryWhenChanged(static x =>
5959
{
60-
using ReadOnlyCollectionPooled<IValidationComponent> validationComponents = new(x);
60+
using ReadOnlyDisposableCollection<IValidationComponent> validationComponents = new(x);
6161
return validationComponents.Count is 0 || validationComponents.All(v => v.IsValid);
62-
})
63-
.Multicast(_validSubject);
62+
});
6463

6564
_isValid = _validSubject
6665
.StartWith(true)
@@ -95,7 +94,7 @@ public IObservable<bool> Valid
9594
/// <summary>
9695
/// Gets get the list of validations.
9796
/// </summary>
98-
public IObservableList<IValidationComponent> Validations { get; private set; }
97+
public IObservableList<IValidationComponent> Validations { get; }
9998

10099
/// <inheritdoc/>
101100
public bool IsValid
@@ -162,10 +161,7 @@ public IValidationText Text
162161
/// <inheritdoc/>
163162
public void Dispose()
164163
{
165-
// Dispose of unmanaged resources.
166164
Dispose(true);
167-
168-
// Suppress finalization.
169165
GC.SuppressFinalize(this);
170166
}
171167

@@ -183,11 +179,8 @@ protected virtual void Dispose(bool disposing)
183179
_validationStatusChange.Dispose();
184180
_validSubject.Dispose();
185181
_validationSource.Clear();
186-
Validations.Dispose();
187182
_validationSource.Dispose();
188-
189-
Validations = null!;
190-
_validationSource = null!;
183+
Validations.Dispose();
191184
}
192185
}
193186

@@ -199,7 +192,7 @@ private void Activate()
199192
}
200193

201194
_isActive = true;
202-
_disposables.Add(_validationConnectable.Connect());
195+
_disposables.Add(_validationObservable.Subscribe(_validSubject));
203196
}
204197

205198
/// <summary>

0 commit comments

Comments
 (0)