MonoGame/MonoGame

Empty Queue Crash in DynamicSoundEffectInstance.PlatformUpdateQueue()

ZachIsAGardner opened this issue · 6 comments

Prerequisites

  • I have verified this issue is present in the develop branch
  • I have searched open and closed issues to ensure it has not already been reported.

MonoGame Version

3.8.1 HOTFIX

Which MonoGame platform are you using?

MonoGame Cross-Platform Desktop Application (mgdesktopgl)

Operating System

Windows 11

Description

My game crashed with the following during regular gameplay:

System.InvalidOperationException: Queue empty.
at System.Collections.Generic.Queue1.ThrowForEmptyQueue() at System.Collections.Generic.Queue1.Dequeue()
at Microsoft.Xna.Framework.Audio.DynamicSoundEffectInstance.PlatformUpdateQueue()
at Microsoft.Xna.Framework.Audio.DynamicSoundEffectInstance.UpdateQueue()
at Microsoft.Xna.Framework.Audio.DynamicSoundEffectInstanceManager.UpdatePlayingInstances()
at Microsoft.Xna.Framework.FrameworkDispatcher.Update()
at Microsoft.Xna.Framework.Game.DoUpdate(GameTime gameTime)
at Microsoft.Xna.Framework.Game.Tick()
at Microsoft.Xna.Framework.SdlGamePlatform.RunLoop()
at Microsoft.Xna.Framework.Game.Run(GameRunBehavior runBehavior)
at GarbanzoQuest.Program.Main()

I see that there was a fix for this here: #7402
However it seems I'm still having this issue on 3.8.1 HOTFIX

Steps to Reproduce

It seemed to happen randomly while the game was playing.

Minimal Example Repo

No response

Expected Behavior

No random crash.

Resulting Behavior

Random crash.

Files

No response

Are you using Threads or Tasks?

I use Tasks to load SFX and Music. I don't think I'm using it anywhere to play anything though.

I wrote some code that hooks into the BufferNeeded event from DynamicSoundEffectInstance in order to have more control over looping and stuff. See my gist below if that provides more context:
https://gist.github.com/ZachIsAGardner/87d34f881dcb60c917e298657e2dfb32

Also, I forked MonoGame and made my own fix(???) to DynamicSoundEffectInstance.OpenAL.cs.

I just wrapped everything in a try/catch and when an error is thrown it calls PlatformStop. I don't know what I'm doing, but I figured it might fix it for now.

private void PlatformUpdateQueue()
{
    try
    {
        // Get the completed buffers
        AL.GetError();
        int numBuffers;
        AL.GetSource(SourceId, ALGetSourcei.BuffersProcessed, out numBuffers);
        ALHelper.CheckError("Failed to get processed buffer count.");

        // Unqueue them
        if (numBuffers > 0)
        {
            AL.SourceUnqueueBuffers(SourceId, numBuffers);
            ALHelper.CheckError("Failed to unqueue buffers.");
            for (int i = 0; i < numBuffers; i++)
            {
                var buffer = _queuedBuffers.Dequeue();
                buffer.Dispose();
            }
        }

        // Raise the event for each removed buffer, if needed
        for (int i = 0; i < numBuffers; i++)
            CheckBufferCount();
    }
    catch(Exception error)
    {
        PlatformStop();
    }
}

Whenever Thread/Task and events are involved with sounds, it makes things hard to know if it's an actual bug or code sync'ing issues in developers' code.

A quick look at your gist makes me thinking that you're loading all sounds each in their own thread at the same time, which might explain the problem.
On a general note, anything involving sound doesn't like at all if multiple sound operations happen at the same time. It's fine to use threads, but it is up to the developer to make sure that no sound operation happen at the same time, loading included.

Regarding your changes in DynamicSoundEffectInstance, this will hide the problem but not fix it. It'd be best to figure out the source problem, and MonoGame shouldn't silence errors.

You're correct, I am loading my 200+ sound effects into memory with one Task each at boot. I'm also loading music similarly, the difference being it only loads the required songs for a level when starting it. It sounds like I shouldn't be though? I've never had a problem with this in the past year it has been this way.

So instead of having one Task per sound effect/song, I could maybe have one Task that loads the requested assets one after the other?

My "fix" of course is not suggested, that part of dev is a little out of my depth and I was just trying to ensure my game wouldn't crash randomly eheh...

So instead of having one Task per sound effect/song, I could maybe have one Task that loads the requested assets one after the other?

That's more advisable, yes. Otherwise there's a probable risk of having OpenAL mixing up buffers internally, which may cascade on other things.

I don't know if in this present instance it is what cascades into breaking DynamicSoundEffectInstance, though.

Your "fix" is likely just postponing an error. I believe that you'll run into other issues later on if you use it. Though fine enough as a local temp patch.