Items not disposed resulting in large memory leak

[jdbriaris]

Describe the bug 🐞

When disposing a subscription to an ObservableCache<,> not all transformed items bound to a ReadOnlyCollection<> are disposed. This can quickly lead to a large memory leak.

Step to reproduce

Using the Dynamic.Trader example application, remove any trade data updates from TradeService by removing any random updates to the source cache (we do this to fix the number of expected items to 5000), e.g., the GenerateTradesAndMaintainCache() becomes

    private IObservable<IChangeSet<Trade, long>> GenerateTradesAndMaintainCache()
    {
        //construct an cache datasource specifying that the primary key is Trade.Id
        return ObservableChangeSet.Create<Trade, long>(cache =>
        {
            //initally load some trades 
            cache.AddOrUpdate(_tradeGenerator.Generate(5_000, true));

            return new CompositeDisposable();
        }, trade=>trade.Id);
    }

Start the application and select to view Paged Data from the menu items to see a sorted/pages list of 5000 trades (the number of trades will now not change).

Profile application memory and observe 5000 TradeProxy objects, as expected.

Close the Paged Data tab and open it again from the main menu any number of times and profile the memory again. The screenshot below is after closing and opening Paged Data a further 10 times.

even after forcing GC we now appear to have a lot of instances of TradeProxy in memory. In the above example we open and closed Paged Data a further 10 times but we don't have 55,000 objects stuck in memory, rather 54,750. Given there are 25 items per page, the numbers seem to suggest that only the paged (visible) items are being disposed when PagedDataViewer is disposed.

Reproduction repository

https://github.com/RolandPheasant/Dynamic.Trader

Expected behavior

When disposing an observable that makes use of paging, all items associated with that observable (both paged and source items) should be disposed.

Screenshots 🖼️

No response

IDE

Visual Studio 2022

Operating system

Windows 10

Version

No response

Device

No response

DynamicData Version

No response

Additional information ℹ️

No response

[jdbriaris]

Note: The above can be alleviated by placing the DisposeMany in PageDataViewer after the Transform, i.e.,

_cleanUp = tradeService.All.Connect()
   .Filter(filter) 
   .Transform(trade => new TradeProxy(trade), new ParallelisationOptions(ParallelType.Ordered, 5))
   .DisposeMany() // Move DisposeMany to here
   .Sort(sort, SortOptimisations.ComparesImmutableValuesOnly)
   .Page(pager)
   .ObserveOn(schedulerProvider.MainThread)
   .Do(changes => PageParameters.Update(changes.Response))
   .Bind(out _data)  
   //.DisposeMany() // This is the original location of DisposeMany     
   .Subscribe();

I question why you would want DisposeMany in the original location after Bind(out _data). Won't this dispose the paged items each time you page the view, but those items are never regenerated (i.e. Transform is only invoked on a filter change) so you could be paging and trying to use disposed objects (if those object are hooked into an observable that has been disposed, this could break functionality). Is this "bug" really the example in Dynamic.Trader not being quite right?

[glennawatson]

I would expect not to have the DisposeMany after the Bind since it's too late in the pipeline.

So looks like a bug in the dynamictrader app.

[RolandPheasant]

I concur. The bug is Dynamic Trader and that DisposeMany should be after Transform.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.