0

I'm trying to make sense of properly disposing of views and viewmodels in an mvvm environment. I hit upon one particular problem with various usercontrols (which in essence are my views) and their associated viewmodels in a docking environment (similar to the sort of thing you might see in visual studio).

I have read countless posts, blogs and questions here and finally found something which I thought was working (indeed it may well be but I just don't understand the process).

In my viewmodel I implement Idiposable and in the view (to which it is set as its datacontext) I ensure that the view calls dispose on its datacontext when it unloads. So view closes and calls dispose on its datacontext (my viewmodel) ensuring that I can clean up any left overs that might hang around and cause a memory leak like event handlers.

One such viewmodel has the following code in the overridden dispose sub:

   ' IDisposable
    Protected Overridable Sub Dispose(disposing As Boolean)
        If Not disposedValue Then
            If disposing Then
                ' TODO: dispose managed state (managed objects).
            End If

            ' TODO: free unmanaged resources (unmanaged objects) and override Finalize() below.
            ' 

            ' 
            If mwvm.CanCallDisposeOnErsSubmissionViewModel Then
                mwvm = Nothing
                subEd = Nothing
                retEd = Nothing
            End If

            ' TODO: set large fields to null.
        End If
        Try

            If Not mwvm.CanCallDisposeOnErsSubmissionViewModel Then
                disposedValue = False
            End If
        Catch ex As NullReferenceException
            disposedValue = True
        End Try
        MessageBox.Show("DisposeValue = " & disposedValue.tostring)
    End Sub

This viewmodel references elements in the mainviewmodel of the application (one of the reasons why I felt it was important to clean it up after use. However the view for which it is the data context gets redrawn if the end user either adds or closes other windows in the docksite in which it sits (hence the need for a conditional statement).

Now this may not be elegant, and it may very well not be the correct way to do this but my question arises from observing the following.

When I choose to actively close the view and the conditional statements allow the clean-up to take place and set the disposed value to true I would have thought that that would mean that the view model had been well and truly closed and disposed of, however I can let the program continue running and hey presto up pops a message box which can only have come from the handler above. So is my model properly disposed off, is this normal behaviour (presumably the garbage collector doing its thing so it can be ignored) or is there a fundamental error on my part somewhere?

Dom Sinclair
  • 2,458
  • 1
  • 30
  • 47
  • Have you seen this question? http://stackoverflow.com/questions/3258064/what-best-practices-for-cleaning-up-event-handler-references – netaholic Oct 23 '15 at 09:30
  • How is your `Dispose()` method (the one from the `IDisposable` interface, the parameterless one) implemented? It has to call `GC.SuppressFinalize(this);` after calling `Dispose(true)`. This will prevent the GC from calling `Dispose()` on the actual garbage collection (you can't control when it happens, GC does it). Check MSDN for correct implementation of the Disposable pattern: https://msdn.microsoft.com/en-us/library/system.gc.suppressfinalize(v=vs.110).aspx – Tseng Oct 23 '15 at 12:09
  • @Tseng Great call, thanks, that's exactly what I'd missed. Mark this up as an answer , not only so that I can accept it but also as an easy way for anyone wondering the same to spot it. – Dom Sinclair Oct 23 '15 at 12:37
  • Feel free to edit the VB.NET Version of it to the answer, as the MSDN doesn't have an example of it – Tseng Oct 23 '15 at 13:03

1 Answers1

1

When using the disposable pattern, you have to call GC.SuppressFinalize(this) inside Dispose() to suppress the GC from calling the Dispose() method when it finalize the object (since you have no control on when GC does it)

From the MSDN on Disposable Pattern:

public class DisposableResourceHolder : IDisposable {

    private SafeHandle resource; // handle to a resource

    public DisposableResourceHolder(){
        this.resource = ... // allocates the resource
    }

    public void Dispose(){
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing){
        if (disposing){
            if (resource!= null) resource.Dispose();
        }
    }
}

To Add to What Tseng has already said, but in relation specifically to VB.

If you Implement IDisposable on a class in VB the following Boiler plate code will be added to the class:

#Region "IDisposable Support"
Private disposedValue As Boolean ' To detect redundant calls

' IDisposable
Protected Overridable Sub Dispose(disposing As Boolean)
    If Not disposedValue Then
        If disposing Then
            ' TODO: dispose managed state (managed objects).
        End If

        ' TODO: free unmanaged resources (unmanaged objects) and override Finalize() below.
        ' TODO: set large fields to null.
    End If
    disposedValue = True
End Sub

' TODO: override Finalize() only if Dispose(disposing As Boolean) above has code to free unmanaged resources.
'Protected Overrides Sub Finalize()
'    ' Do not change this code.  Put cleanup code in Dispose(disposing As Boolean) above.
'    Dispose(False)
'    MyBase.Finalize()
'End Sub

' This code added by Visual Basic to correctly implement the disposable pattern.
Public Sub Dispose() Implements IDisposable.Dispose
    ' Do not change this code.  Put cleanup code in Dispose(disposing As Boolean) above.
    Dispose(True)
    ' TODO: uncomment the following line if Finalize() is overridden above.
    ' GC.SuppressFinalize(Me)
End Sub

Stackoverflow actually highlights code in a more readable fashion than the visual studio editor does. I had added the bits that I knew needed to be disposed to the Overridable Sub Dispose. I had taken note of the comment in that section to uncomment the Ovveride Finalize section. What I had missed was to go on and uncomment the GC.SuppressFinalize(Me) line in the actual Dispose method. A stupid mistake on my part the only excuse I can offer is that the green comments blend things together too well.

Hopefully this will be of help to others.

Dom Sinclair
  • 2,458
  • 1
  • 30
  • 47
Tseng
  • 61,549
  • 15
  • 193
  • 205