Skip to content

Commit 79dd805

Browse files
ionite34ionite34
authored
Merge pull request LykosAI#751 from ionite34/backport/main/pr-750
[dev to main] backport: Add IDisposable return for RelayPropertyFor subscriptions, and use dispose for transients (750)
2 parents bf7f0b8 + d55886a commit 79dd805

File tree

11 files changed

+218
-42
lines changed

11 files changed

+218
-42
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning 2.0](https://semver.org/spec/v2
88
## v2.11.6
99
### Fixed
1010
- Fixed incorrect IPAdapter download links in the HuggingFace model browser
11+
- Fixed potential memory leak of transient controls (Inference Prompt and Output Image Viewer) not being garbage collected due to event subscriptions
1112

1213
## v2.11.5
1314
### Added

‎StabilityMatrix.Avalonia/ViewModels/Base/DisposableLoadableViewModelBase.cs‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
using System;
2+
using System.Reactive.Disposables;
3+
using JetBrains.Annotations;
4+
5+
namespace StabilityMatrix.Avalonia.ViewModels.Base;
6+
7+
public abstract class DisposableLoadableViewModelBase : LoadableViewModelBase, IDisposable
8+
{
9+
private readonly CompositeDisposable instanceDisposables = new();
10+
11+
/// <summary>
12+
/// Adds a disposable to be disposed when this view model is disposed.
13+
/// </summary>
14+
/// <param name="disposable">The disposable to add.</param>
15+
protected void AddDisposable([HandlesResourceDisposal] IDisposable disposable)
16+
{
17+
instanceDisposables.Add(disposable);
18+
}
19+
20+
/// <summary>
21+
/// Adds disposables to be disposed when this view model is disposed.
22+
/// </summary>
23+
/// <param name="disposables">The disposables to add.</param>
24+
protected void AddDisposable([HandlesResourceDisposal] params IDisposable[] disposables)
25+
{
26+
foreach (var disposable in disposables)
27+
{
28+
instanceDisposables.Add(disposable);
29+
}
30+
}
31+
32+
/// <summary>
33+
/// Adds a disposable to be disposed when this view model is disposed.
34+
/// </summary>
35+
/// <param name="disposable">The disposable to add.</param>
36+
/// <typeparam name="T">The type of the disposable.</typeparam>
37+
/// <returns>The disposable that was added.</returns>
38+
protected T AddDisposable<T>([HandlesResourceDisposal] T disposable)
39+
where T : IDisposable
40+
{
41+
instanceDisposables.Add(disposable);
42+
return disposable;
43+
}
44+
45+
/// <summary>
46+
/// Adds disposables to be disposed when this view model is disposed.
47+
/// </summary>
48+
/// <param name="disposables">The disposables to add.</param>
49+
/// <typeparam name="T">The type of the disposables.</typeparam>
50+
/// <returns>The disposables that were added.</returns>
51+
protected T[] AddDisposable<T>([HandlesResourceDisposal] params T[] disposables)
52+
where T : IDisposable
53+
{
54+
foreach (var disposable in disposables)
55+
{
56+
instanceDisposables.Add(disposable);
57+
}
58+
59+
return disposables;
60+
}
61+
62+
protected virtual void Dispose(bool disposing)
63+
{
64+
if (disposing)
65+
{
66+
instanceDisposables.Dispose();
67+
}
68+
}
69+
70+
public void Dispose()
71+
{
72+
Dispose(true);
73+
GC.SuppressFinalize(this);
74+
}
75+
}

‎StabilityMatrix.Avalonia/ViewModels/Base/DisposableViewModelBase.cs‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
using System;
2+
using System.Reactive.Disposables;
3+
using JetBrains.Annotations;
4+
5+
namespace StabilityMatrix.Avalonia.ViewModels.Base;
6+
7+
public abstract class DisposableViewModelBase : ViewModelBase, IDisposable
8+
{
9+
private readonly CompositeDisposable instanceDisposables = new();
10+
11+
/// <summary>
12+
/// Adds a disposable to be disposed when this view model is disposed.
13+
/// </summary>
14+
/// <param name="disposable">The disposable to add.</param>
15+
protected void AddDisposable([HandlesResourceDisposal] IDisposable disposable)
16+
{
17+
instanceDisposables.Add(disposable);
18+
}
19+
20+
/// <summary>
21+
/// Adds disposables to be disposed when this view model is disposed.
22+
/// </summary>
23+
/// <param name="disposables">The disposables to add.</param>
24+
protected void AddDisposable([HandlesResourceDisposal] params IDisposable[] disposables)
25+
{
26+
foreach (var disposable in disposables)
27+
{
28+
instanceDisposables.Add(disposable);
29+
}
30+
}
31+
32+
/// <summary>
33+
/// Adds a disposable to be disposed when this view model is disposed.
34+
/// </summary>
35+
/// <param name="disposable">The disposable to add.</param>
36+
/// <typeparam name="T">The type of the disposable.</typeparam>
37+
/// <returns>The disposable that was added.</returns>
38+
protected T AddDisposable<T>([HandlesResourceDisposal] T disposable)
39+
where T : IDisposable
40+
{
41+
instanceDisposables.Add(disposable);
42+
return disposable;
43+
}
44+
45+
/// <summary>
46+
/// Adds disposables to be disposed when this view model is disposed.
47+
/// </summary>
48+
/// <param name="disposables">The disposables to add.</param>
49+
/// <typeparam name="T">The type of the disposables.</typeparam>
50+
/// <returns>The disposables that were added.</returns>
51+
protected T[] AddDisposable<T>([HandlesResourceDisposal] params T[] disposables)
52+
where T : IDisposable
53+
{
54+
foreach (var disposable in disposables)
55+
{
56+
instanceDisposables.Add(disposable);
57+
}
58+
59+
return disposables;
60+
}
61+
62+
protected virtual void Dispose(bool disposing)
63+
{
64+
if (disposing)
65+
{
66+
instanceDisposables.Dispose();
67+
}
68+
}
69+
70+
public void Dispose()
71+
{
72+
Dispose(true);
73+
GC.SuppressFinalize(this);
74+
}
75+
}

‎StabilityMatrix.Avalonia/ViewModels/Base/InferenceGenerationViewModelBase.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ RunningPackageService runningPackageService
9393
ClientManager = inferenceClientManager;
9494

9595
ImageGalleryCardViewModel = vmFactory.Get<ImageGalleryCardViewModel>();
96-
ImageFolderCardViewModel = vmFactory.Get<ImageFolderCardViewModel>();
96+
ImageFolderCardViewModel = AddDisposable(vmFactory.Get<ImageFolderCardViewModel>());
9797

9898
GenerateImageCommand.WithConditionalNotificationErrorHandler(notificationService);
9999
}

‎StabilityMatrix.Avalonia/ViewModels/Base/InferenceTabViewModelBase.cs‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@
3333
namespace StabilityMatrix.Avalonia.ViewModels.Base;
3434

3535
public abstract partial class InferenceTabViewModelBase
36-
: LoadableViewModelBase,
37-
IDisposable,
36+
: DisposableLoadableViewModelBase,
3837
IPersistentViewProvider,
3938
IDropTarget
4039
{
@@ -158,21 +157,16 @@ private async Task DebugLoadViewState()
158157
}
159158
}
160159

161-
protected virtual void Dispose(bool disposing)
160+
protected override void Dispose(bool disposing)
162161
{
162+
base.Dispose(disposing);
163+
163164
if (disposing)
164165
{
165166
((IPersistentViewProvider)this).AttachedPersistentView = null;
166167
}
167168
}
168169

169-
/// <inheritdoc />
170-
public void Dispose()
171-
{
172-
Dispose(true);
173-
GC.SuppressFinalize(this);
174-
}
175-
176170
/// <summary>
177171
/// Loads image and metadata from a file path
178172
/// </summary>

‎StabilityMatrix.Avalonia/ViewModels/Inference/ImageFolderCardViewModel.cs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ namespace StabilityMatrix.Avalonia.ViewModels.Inference;
3636
[View(typeof(ImageFolderCard))]
3737
[ManagedService]
3838
[Transient]
39-
public partial class ImageFolderCardViewModel : ViewModelBase
39+
public partial class ImageFolderCardViewModel : DisposableViewModelBase
4040
{
4141
private readonly ILogger<ImageFolderCardViewModel> logger;
4242
private readonly IImageIndexService imageIndexService;
@@ -83,11 +83,13 @@ INotificationService notificationService
8383
.Bind(LocalImages)
8484
.Subscribe();
8585

86-
settingsManager.RelayPropertyFor(
87-
this,
88-
vm => vm.ImageSize,
89-
settings => settings.InferenceImageSize,
90-
delay: TimeSpan.FromMilliseconds(250)
86+
AddDisposable(
87+
settingsManager.RelayPropertyFor(
88+
this,
89+
vm => vm.ImageSize,
90+
settings => settings.InferenceImageSize,
91+
delay: TimeSpan.FromMilliseconds(250)
92+
)
9193
);
9294
}
9395

‎StabilityMatrix.Avalonia/ViewModels/Inference/InferenceTextToImageViewModel.cs‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ RunningPackageService runningPackageService
8383
samplerCard.DenoiseStrength = 1.0d;
8484
});
8585

86-
PromptCardViewModel = vmFactory.Get<PromptCardViewModel>();
86+
PromptCardViewModel = AddDisposable(vmFactory.Get<PromptCardViewModel>());
87+
8788
BatchSizeCardViewModel = vmFactory.Get<BatchSizeCardViewModel>();
8889

8990
ModulesCardViewModel = vmFactory.Get<StackEditableCardViewModel>(modulesCard =>
@@ -108,13 +109,15 @@ RunningPackageService runningPackageService
108109
);
109110

110111
// When refiner is provided in model card, enable for sampler
111-
ModelCardViewModel
112-
.WhenPropertyChanged(x => x.IsRefinerSelectionEnabled)
113-
.Subscribe(e =>
114-
{
115-
SamplerCardViewModel.IsRefinerStepsEnabled =
116-
e.Sender is { IsRefinerSelectionEnabled: true, SelectedRefiner: not null };
117-
});
112+
AddDisposable(
113+
ModelCardViewModel
114+
.WhenPropertyChanged(x => x.IsRefinerSelectionEnabled)
115+
.Subscribe(e =>
116+
{
117+
SamplerCardViewModel.IsRefinerStepsEnabled =
118+
e.Sender is { IsRefinerSelectionEnabled: true, SelectedRefiner: not null };
119+
})
120+
);
118121
}
119122

120123
/// <inheritdoc />

‎StabilityMatrix.Avalonia/ViewModels/Inference/PromptCardViewModel.cs‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ namespace StabilityMatrix.Avalonia.ViewModels.Inference;
2828
[View(typeof(PromptCard))]
2929
[ManagedService]
3030
[Transient]
31-
public partial class PromptCardViewModel : LoadableViewModelBase, IParametersLoadableState, IComfyStep
31+
public partial class PromptCardViewModel
32+
: DisposableLoadableViewModelBase,
33+
IParametersLoadableState,
34+
IComfyStep
3235
{
3336
private readonly IModelIndexService modelIndexService;
3437
private readonly ISettingsManager settingsManager;
@@ -75,11 +78,13 @@ SharedState sharedState
7578
vm.AvailableModules = [typeof(PromptExpansionModule)];
7679
});
7780

78-
settingsManager.RelayPropertyFor(
79-
this,
80-
vm => vm.IsAutoCompletionEnabled,
81-
settings => settings.IsPromptCompletionEnabled,
82-
true
81+
AddDisposable(
82+
settingsManager.RelayPropertyFor(
83+
this,
84+
vm => vm.IsAutoCompletionEnabled,
85+
settings => settings.IsPromptCompletionEnabled,
86+
true
87+
)
8388
);
8489
}
8590

‎StabilityMatrix.Core/Services/ISettingsManager.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public interface ISettingsManager
7373
/// <summary>
7474
/// Register a source observable object and property to be relayed to Settings
7575
/// </summary>
76-
void RelayPropertyFor<T, TValue>(
76+
IDisposable RelayPropertyFor<T, TValue>(
7777
T source,
7878
Expression<Func<T, TValue>> sourceProperty,
7979
Expression<Func<Settings, TValue>> settingsProperty,

‎StabilityMatrix.Core/Services/SettingsManager.cs‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
using System.ComponentModel;
1+
using System.ComponentModel;
22
using System.Diagnostics.CodeAnalysis;
33
using System.Linq.Expressions;
4+
using System.Reactive.Disposables;
5+
using System.Reactive.Linq;
46
using System.Reflection;
57
using System.Text.Json;
68
using AsyncAwaitBestPractices;
@@ -162,7 +164,7 @@ public void Transaction<TValue>(Expression<Func<Settings, TValue>> expression, T
162164
}
163165

164166
/// <inheritdoc />
165-
public void RelayPropertyFor<T, TValue>(
167+
public IDisposable RelayPropertyFor<T, TValue>(
166168
T source,
167169
Expression<Func<T, TValue>> sourceProperty,
168170
Expression<Func<Settings, TValue>> settingsProperty,
@@ -180,7 +182,7 @@ public void RelayPropertyFor<T, TValue>(
180182
var sourceTypeName = source.GetType().Name;
181183

182184
// Update source when settings change
183-
SettingsPropertyChanged += (sender, args) =>
185+
void OnSettingsPropertyChanged(object? sender, RelayPropertyChangedEventArgs args)
184186
{
185187
if (args.PropertyName != settingsPropertyPath)
186188
return;
@@ -197,10 +199,10 @@ public void RelayPropertyFor<T, TValue>(
197199
);
198200

199201
sourceInstanceAccessor.Set(source, settingsAccessor.Get(Settings));
200-
};
202+
}
201203

202204
// Set and Save settings when source changes
203-
source.PropertyChanged += (sender, args) =>
205+
void OnSourcePropertyChanged(object? sender, PropertyChangedEventArgs args)
204206
{
205207
if (args.PropertyName != sourcePropertyPath)
206208
return;
@@ -240,13 +242,32 @@ public void RelayPropertyFor<T, TValue>(
240242
sender,
241243
new RelayPropertyChangedEventArgs(settingsPropertyPath, true)
242244
);
243-
};
245+
}
244246

245-
// Set initial value if requested
246-
if (setInitial)
247+
var subscription = Disposable.Create(() =>
248+
{
249+
source.PropertyChanged -= OnSourcePropertyChanged;
250+
SettingsPropertyChanged -= OnSettingsPropertyChanged;
251+
});
252+
253+
try
247254
{
248-
sourceInstanceAccessor.Set(settingsAccessor.Get(Settings));
255+
SettingsPropertyChanged += OnSettingsPropertyChanged;
256+
source.PropertyChanged += OnSourcePropertyChanged;
257+
258+
// Set initial value if requested
259+
if (setInitial)
260+
{
261+
sourceInstanceAccessor.Set(settingsAccessor.Get(Settings));
262+
}
249263
}
264+
catch
265+
{
266+
subscription.Dispose();
267+
throw;
268+
}
269+
270+
return subscription;
250271
}
251272

252273
/// <inheritdoc />

0 commit comments

Comments
 (0)