last day (14 days later) » 

8:02 AM
3
A: NamedScope and garbage collection

BatteryBackupUnitTL;DR short answer: add INotifyWhenDisposed to your View. Dispose the view. This will lead to ninject automatically disposing all stuff bound InNamedScope plus also ninject will un-reference these objects. This will lead to (eventual) garbage collection (unless you're hanging on to strong refere...

 
Excellent explanation, but alas, this did not solve my problem. I added an OnActivation callback on the ViewModel binding and retrieved the scope object for inspection: It does not seem to be the View which acts as the scope object, but rather a DisposeNotifyingObject, which already implements INotifyWhenDisposed.
As a side note, I thought that explicitly releasing the view with Kernel.Release(), would clean up the named scope that the View binding defined.
If I dispose the scope object (the DisposeNotifyingObject mentioned above) the ViewModel is garbage collected and the unit test succeeds.
 
@AndreasAppelros yes you're right as the scope object ninject is not actually using the object bound with DefinesNamedScope(...). However, as the ninject integration test shows it'll still work if the object bound (View in your case) implements INotifyWhenDisposed. Did you give it a try?
@AndreasAppelros kernel.Release doesn't even call Dispose on the object. It just removes it from the cache.
 
@BatteryBackupUnit Since View is in Call scope, kernel.Release() does actually call Dispose on the object. Just checked in the debugger to be sure. Thats true for INotifyWhenDisposed as well. Yes, I tried letting the View implement INotifyWhenDisposed, the ViewModel does not get GC:d.
 
@AndreasAppelros then what is/are the ViewModel's path/-s to the GCRoot?
 
Retention path of ViewModel

<D> GarbageCollectionCachePruner.caches ->
<D> List<IPruneable>._items ->
<D> IPruneable[] at [1] ->
<D> Cache.entries ->
<D> Dictionary<Object, Multimap<IBindingConfiguration, Cache+CacheEntry>>.entries ->
<D> Dictionary+Entry<Object, Multimap<IBindingConfiguration, Cache+CacheEntry>>[] at [0].value ->
<D> Multimap<IBindingConfiguration, Cache+CacheEntry>._items ->
<D> Dictionary<IBindingConfiguration, ICollection<Cache+CacheEntry>>.entries ->
<D> Dictionary+Entry<IBindingConfiguration, ICollection<Cache+CacheEntry>>[] at [0].value ->
 
8:06 AM
can you check and see whether it has any effect if you call kernel.Release(viewModel)?
are you also using the ContextPreservation Extension?
also what's the return value of kernel.Release(viewModel)?
 
Yes, ContextPreservation is loaded. Releasing the ViewModel returns true and makes the ViewModel be GC:d.
One way to solve it would be to dispose of the scope object on View deactivation. Do you think that could lead to other problems?
 
8:22 AM
(i too have the ContetPreservation extension loaded)
ok so something seems different with your code than with mine. If you look at the code that i posted, release returns false.
 
Yes, my View binding is InCallScope.
And my ViewModel is not IDisposable.
 
InCallScope? In the unit test you posted it's not.
ViewModel being IDisposable should not matter. But i can verify on my end as well.
ViewModel not being IDisposable is still get false from kernel.Release(viewModel)
so it seems to be related to InCallScope.
Adding InCallScope does lead to the test passing on my end as well
i suspect its the combination of InCallScope + ContextPreservationExtension that leads to this issue. However, you cannot get rid of the ContextPreservationExtension if you need the DependencyCreation nuget package..
(of course removing Context preservation might also complicate things in a lot of other places in your code)
 
8:48 AM
Sorry, my bad, I must have modified the UnitTest afterwards when finding out that Disposed wasn't called when I called Kernel.Release().
But what do you think of Disposing the scope object in the View:s OnDeactivation?
 
@AndreasAppelros well if it works it works ;-) I would prefer disposing the View though because it seems to me it would read more "naturally" and doesn't need access to kernel.Release. However if this doesn't work... it's probably the easiest way to go
i remember there being a bug / issue with InCallScope, that's why at the previous company we wrote our own implementation. We discussed it with the maintainer and i think we agreed ninject's implementation should be changed / fixed. however, it seems that we failed to post it as an issue on github. Thus i don't know whether it was related to your issue or not.
It took us several days to find the actual issue and write an implementation which works correctly... so given that, if the OnDeactivation hack works properly, i would just go for it...
 
Yes, Dispose would be a better way to go. It seems to work when the form is closed, then all is GC:d, so it's just when I want to add/remove components while the form is open. I added a Release method to my factory, so thats the only place referencing IResolutionRoot.
 
ok if it works i suggest you should post it as an answer and accept it.
alternatively: Why do you need InCallScope? Usually this can be replaced by another named scope or the likes...
 
What I wanted was for Ninject to release the ViewModel, which didn't happen when the View was Disposed, so I tried to Release the View instead, and then it couldn't be InTransientScope. InCallScope seemed to be the easiest way. The View is kind of the scope root (for the ViewModel, which is injected in both the View and Presenter). But I may have gotten to deep into my solution so that I'm missing the bigger picture.
I thought that Ninject didn't know that the scope could be GC:d and was hoping that Releasing the View, which had defined the named scope, would solve it.
I guess it would be possible to use the view as a custom scope, maybe...
 
9:08 AM
what? :D if it's just about releasing the view model, then you should just remove the InCallScope and have View : INotifyWhenDisposed. No kernel.Release. Just do view.Dispose() and all is good.
just as i've shown to work in the last piece of code in my answer: stackoverflow.com/a/32120527/684096
without the InCallScope it works perfectly fine.
note: doesn't require IDisposable on the ViewModel
 
But will the View and Presenter get the same instance of the ViewModel?
 
sure, that's what you got the named scope for
ok let me check first :D
 
Ok. Will try, and see what dotMemory says.
 
it works.
 
The View and Presenter gets GC:d for me, but not the ViewModel.
 
9:13 AM
can you just upload your whole code again?
 
Same retention path as posted earlier.
 
because if there's not InCallScope my unit test returns false for kernel.Release(viewModel)
which is not a 100% guarantee but still...
 
In the chat?
 
yeah that's ok
 
using System;
using System.Threading;
using FluentAssertions;
using JetBrains.dotMemoryUnit;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Ninject;
using Ninject.Extensions.DependencyCreation;
using Ninject.Extensions.NamedScope;
using Ninject.Infrastructure.Disposal;
using Ninject.Syntax;

namespace UnitTestProject
{
    [TestClass]
    [DotMemoryUnit(FailIfRunWithoutSupport = false)]
    public class UnitTest1
    {
        [TestCleanup]
        public void Cleanup()
        {

        }
Wait, there's some garbage code as well, let me cleanup.
Hm, my kernel.Release(viewModel) returns true.
 
9:20 AM
yes. So i guess your code still differs from mine ;-)
aah formatting is bad
not better. how do i retain formatting?
 
There was a fixed font button next to my send button when I pasted code.
 
    namespace UnitTestProject
    {
        using FluentAssertions;
        using Ninject;
        using Ninject.Extensions.DependencyCreation;
        using Ninject.Extensions.NamedScope;
        using Ninject.Infrastructure.Disposal;
        using System;
        using Xunit;

        public class UnitTest1
        {
            [Fact]
            public void TestMethod()
            {
                // Arrange
                var kernel = new StandardKernel();
                const string namedScope = "namedScope";
thanks. didn't wait for UI to update :D
can you just add the dotCover.unit test into my code for a test?
of course you'll need the "sub method" again so there's no local variables left over...
 
Will do
For me it fails here:
kernel.Release(view.ViewModel).Should().BeFalse();
 
? with the exact same code?
 
Yeah, I just pasted the method, but I'm using another framework so [TestMethod] instead of [Fact]
Which Ninject version are you using?
 
9:27 AM
3.2.2.0
latest stable for everything i guess
context preservation: 3.2.0.0
DependencyCreation: 3.2.0.0
Factory: 3.2.1.0
Interception: 3.2.0.0
Interception.DynamicProxy: 3.2.0.0
NamedScope: 3.2.0.0
Castle.Core: 3.3.1
if you can't find the reason, can you create a repro-solution? Without dotcover.unit, b/c it seems you can repro it without it (
kernel.Release(view.ViewModel).Should().BeFalse(); fails)
 
Ok. I have 3.2.0.0 for everything.
 
ninject as well?
 
because there's 3.2.2
then i'd suggest you try updating it. Not sure if it's going to change anything.. but it's easy enough to test.
 
9:49 AM
Tried updating, but it still failed, so I created a new solution/project and this succeeds. Will attach dotMemory and see if this destroys the test again. :)
 
ok good
(well not so good but at least we're making progress!)
 
This works.
Now I just have to understand what the difference is. :) Thanks a bunch for all help, I will investigate further and then tag your answer as correct!
Ok, now I know. It was my mistake. I didn't implement Dispose so that it called its handler, so Ninject couldn't listen to the Dispose event.
Sorry for taking up so much of your time, you were right all along, I just didn't understand how to implement INotifyWhenDisposed correctly. :(
 
10:35 AM
Should I remove our discussion in StackOverflow and just keep my last comment or just leave it as is (newbie question :) )?
 
i've edited your question to remove the InCallScope (b/c otherwise the solution really doesn't work)
now if you want to remove the discussion or not is up to you
in my opinion it's ok
if someone else has a look at it, it shows, that they have to take really good care to do everything exactly the same way or it won't work...
but if you remove your comments i'll delete mine as well.
oh: you should at least leave the comment stating why you missed to adapt the implementation of Dispose (b/c the user control already implemented it).
 
10:53 AM
Yeah, that was what I thought to leave. But if you mark that comment as useful it will raise to the top, right? Maybe thats better. I managed to delete the link to this chat though, maybe that wasn't so good? Or are others prohibited to view the chat?
managed = by mistake :)
 

last day (14 days later) »