Content does not unload #848

Closed
Nezz opened this Issue Oct 7, 2012 · 12 comments

Projects

None yet

5 participants

@Nezz
Member
Nezz commented Oct 7, 2012

We are currently working on switching levels in our game. We use a contentmanager for the gamescreen, which should be unloaded when a new level is loaded. After several level changes, we receive a low memory crash.
I have noticed that the destructors of the types we loaded using the content.load were not called and the levels were not unloaded properly. This does not happen in XNA on Windows or WP, but happens on my iPad.
Calling Content.Unload manually helps, but it is just a workaround.

Owner

How are you destructing the ContentManager? Are you just waiting for the finalizer to fire?

Looking at the code...

https://github.com/mono/MonoGame/blob/develop3d/MonoGame.Framework/Content/ContentManager.cs

... looks like the finalizer is doing Dispose(false) which keeps it from calling Unload(). Seems like we should just put Unload() in the finalizer.

Member
Nezz commented Oct 7, 2012

Yep, we get rid of all the references and call GC.Collect().
This seems the be the issue:
private static List ContentManagers = new List();

Contributor

Even in XNA, you should always all Dispose on the ContentManager when you
want the content to unload.

Member
Nezz commented Oct 7, 2012

That's wrong. We have a game that is unloading fine both on Windows 7 and WP7 without calling dispose.
We should just let the garbage collector do the job (why should I take out the trash myself if the trashman comes to my house?). Currently, MonoGame does not let the GC do its job.
I see that it was added for Android, but in my opinion, the way the whole thing is handled is plain wrong.
See:
#738

Contributor

If a class implements IDisposable, you should always call Dispose() on it
when you are finished with it. That's not a MonoGame thing. That is how
you are supposed to do it in .NET in general.

You will see in ContentManager.Dispose() that it has a call to
RemoveContentManager(this). That removes it from the list if you correctly
call Dispose().

The list is there so that MonoGame can reload assets when required. This
may change behaviour a bit when compared with XNA, but it is something that
we needed to do support the platforms such as Android where GPU assets are
commonly discarded by the OS.

Contributor

To assist with this, we could change the list from

List

to

List

This will allow the ContentManager to be garbage collected when all other
references to it have been removed. It doesn't remove the responsibility
to call Dispose though.

Contributor

If you do this, then you need to make sure that you check the value of 'target' in the WeakReference for unexpected nulls. the GC will collect your object as it sees fit when you use the WeakReference. Be wary. I've used this on large collections with unexpected results. The "WeakReference" object is not collected, only its target...

Contributor

For sure. That's a requirement of using WeakReference.

Member
Nezz commented Oct 8, 2012

@slygamer It is called IDisposABLE, not IMustBeDisposed.
From MSDN: "The primary use of this interface is to release unmanaged resources. The garbage collector automatically releases the memory allocated to a managed object when that object is no longer used. However, it is not possible to predict when garbage collection will occur. Furthermore, the garbage collector has no knowledge of unmanaged resources such as window handles, or open files and streams."
So resources will be released by the GC anyway, but we don't know when (there are no unmanaged resources in ContentManager). That's why we call GC.Collect() after unreferencing.

I like the idea of WeakReferences. It should allow us to reuse ContentManagers via Unload().

Contributor

The content manager is (indirectly) full of unmanaged resources. OpenGL
textures, vertex buffers, index buffers, audio buffers to name a few.
That's why it implements IDisposable.

Changing the internal list to use WeakReferences shouldn't have any bearing
on reusing a ContentManager after calling Unload().

Member
Nezz commented Oct 8, 2012

See the first answer here regarding the proper use of IDisposable:
http://stackoverflow.com/questions/538060/proper-use-of-the-idisposable-interface

We should never rely on the user calling Dispose.
I will rewrite this later today.

Contributor

I never said you should rely on the user calling Dispose(). Indeed, if you
look at the several examples in the MSDN documentation, and ultimately the
result that SO answer came to, is to override the finalizer in case the
developer forgot to call Dispose().

This thread should end now as we have a solution for your initial problem.
How you interpret the use of the Dispose() pattern is a matter of
programming practice.

@KonajuGames KonajuGames added a commit that referenced this issue Oct 10, 2012
@KonajuGames KonajuGames ContentManager uses WeakReference for list
Change ContentManager's internal list of content managers to a list of WeakReference so content managers can be freed by game code releasing all references to them.  Fixes issue #848.
1d5ec56
@dellis1972 dellis1972 closed this Oct 10, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment