GL Object Disposal Wrappers are not thread-safe

[renaudbedard]

The GL Object Disposal Wrapper methods of GLDevice (AddDisposeTexture, AddDisposeRenderbuffer, ...) use Queue<T> objects to store buffers to dispose if attempted to dispose on other threads, and in SwapBuffers(), FNA iterates over the queues to dispose them inline.

It's possible (and I hit it a bunch of times in FEZ) that a queue gets added to and iterated/dequeued at the same time on two threads, which breaks the queue and a bunch of "null" elements appear in it.

This can be fixed by using ConcurrentQueue<T>, a lock-free concurrent queue implementation. Here's how I did it locally :

#region Private Graphics Object Disposal Queues

private ConcurrentQueue<IGLTexture> GCTextures = new ConcurrentQueue<IGLTexture>();
private ConcurrentQueue<IGLRenderbuffer> GCDepthBuffers = new ConcurrentQueue<IGLRenderbuffer>();
private ConcurrentQueue<IGLBuffer> GCVertexBuffers = new ConcurrentQueue<IGLBuffer>();
private ConcurrentQueue<IGLBuffer> GCIndexBuffers = new ConcurrentQueue<IGLBuffer>();
private ConcurrentQueue<IGLEffect> GCEffects = new ConcurrentQueue<IGLEffect>();
private ConcurrentQueue<IGLQuery> GCQueries = new ConcurrentQueue<IGLQuery>();

#endregion

And in SwapBuffers()...

IGLTexture gcTexture;
while (GCTextures.TryDequeue(out gcTexture))
{
   DeleteTexture(gcTexture);
}

IGLRenderbuffer gcDepthBuffer;
while (GCDepthBuffers.TryDequeue(out gcDepthBuffer))
{
   DeleteRenderbuffer(gcDepthBuffer);
}

IGLBuffer gcBuffer;
while (GCVertexBuffers.TryDequeue(out gcBuffer))
{
   DeleteVertexBuffer(gcBuffer);
}
while (GCIndexBuffers.TryDequeue(out gcBuffer))
{
   DeleteIndexBuffer(gcBuffer);
}

IGLEffect gcEffect;
while (GCEffects.TryDequeue(out gcEffect))
{
   DeleteEffect(gcEffect);
}

IGLQuery gcQuery;
while (GCQueries.TryDequeue(out gcQuery))
{
   DeleteQuery(gcQuery);
}

The wrapper methods don't change since Enqueue() has the same API.

[flibitijibibo]

https://github.com/flibitijibibo/FNA/commit/f9f58e102e60885b24378475a5cc05c7717b7323

[renaudbedard]

Minor, but testing Count does not guarantee that the count will be the same right after... TryDequeue should be just fine, it will return false if the queue's empty.

[flibitijibibo]

Looking at the .NET reference source (which upstream Mono now uses) it looks like the benefit of checking Count is lost now - even Count does some dirty work to check the actual count, unlike the older mscorlibs. So nnnnnnwhatevs!

https://github.com/flibitijibibo/FNA/commit/001e269924724e43d52750ee3d007b6911b1fc5d