Scott Hanselman

I added a new feature to BabySmash during lunch, so that if your (baby's) mouse wheel is over a shape and they scroll the wheel, the system will play a sound and zoom that object in or out. The mouse wheel events come REALLY fast, as do most mouse events.

The general idea is this. I've got the PInvoke/DllImport call to the PlaySound API and a couple of helper methods. If the WAV wasn't cached, I'd go get it and store it away. All this code was kind of written on auto-pilot, you know? It's called very quickly in the MouseWheel event and works fine...until it totally doesn't work at all.

I found that when I wheeled the mouse REALLY fast, sometimes it'd get a nasty burst of loud static instead of the nice WAV file playing as I expected.

I store my WAV files inside the resources of BabySmash.exe (for now) so I just hold them in memory. Initially I would pull them out of the resource every time, but then I added some basic caching. (I probably should have used Chad and Jeremy's cache, but anyway)

[DllImport("winmm.dll")]
public static extern bool PlaySound(byte[] data, IntPtr hMod, UInt32 dwFlags);

public static void PlayWavResource(string wav)
{
    byte[] b = GetWavResource(wav);
    PlaySound(b, IntPtr.Zero, SND_ASYNC | SND_MEMORY);
}

public static void PlayWavResourceYield(string wav)
{
    byte[] b = GetWavResource(wav);
    PlaySound(b, IntPtr.Zero, SND_ASYNC | SND_MEMORY | SND_NOSTOP);
}

private static byte[] GetWavResource(string wav)
{
    //TODO: Is this valid double-check caching?
    byte[] b = null;
    if (cachedWavs.ContainsKey(wav))
        b = cachedWavs[wav];
    if (b == null)
    {
        lock (cachedWavsLock)
        {
            // get the namespace 
            string strNameSpace = Assembly.GetExecutingAssembly().GetName().Name;

            // get the resource into a stream
            using (Stream str = Assembly.GetExecutingAssembly().GetManifestResourceStream(strNameSpace + wav))
            {
                if (str == null)
                    throw new ArgumentException(wav + " not found!");
                // bring stream into a byte array
                var bStr = new Byte[str.Length];
                str.Read(bStr, 0, (int)str.Length);
                cachedWavs.Add(wav, bStr);
                return bStr;
            }
        }
    }
    return b;
}

Anyway, I kind of forgot that byte was a value type and in a chat this afternoon Larry made this comment. You might remember that the man responsible for the PlaySound() API is none other than Larry Osterman, who I interviewed last year. Here's our chat transcript:

Larry Osterman‎‎:
My guess is that you're deleting the array b before the PlaySound has completed.
or rather the CLR is.

‎‎Scott Hanselman:
even though it's on the stack?
ah
I get it
the GC is getting to it

‎‎Larry Osterman‎‎:
when you say snd_async, it queues the actual playsound operation to a worker thread.
Yup, GC makes it go away.

When I started going really fast with dozens of calls to PlaySound() a second, I was piling these up and eventually hit the point where one byte[] that was being played would disappear (get garbage collected) and I'd hear the sound of zeros being played. Which sounds much like static. (kidding) ;) I could have made the sound play synchronously, but that doesn't fit well with BabySmash's free-form maniacal button pressing.

Larry suggested I copy the WAV files to a temporary location so they'd be "pinned" down, as there wasn't really a good way to pin these in memory that either of us could come up with. Here's what I did. I grabbed a TempFileName, put the WAV files on disk there and switched the call to PlaySound to the filename overloaded version, rather than the byte[] version. I use TempFileCollection which is helpful because it automatically tries to delete the temporary files when its finalizer runs.

[DllImport("winmm.dll", SetLastError = true)]
static extern bool PlaySound(string pszSound, IntPtr hmod, UInt32 fdwSound);

public void PlayWavResource(string wav)
{
    string s = GetWavResource(wav);
    PlaySound(s, IntPtr.Zero, SND_ASYNC);
}

public void PlayWavResourceYield(string wav)
{
    string s = GetWavResource(wav);
    PlaySound(s, IntPtr.Zero, SND_ASYNC | SND_NOSTOP);
}

TempFileCollection tempFiles = new TempFileCollection();

private string GetWavResource(string wav)
{
    //TODO: Is this valid double-check caching?
    string retVal = null;
    if (cachedWavs.ContainsKey(wav))
        retVal = cachedWavs[wav];
    if (retVal == null)
    {
        lock (cachedWavsLock)
        {
            // get the namespace 
            string strNameSpace = Assembly.GetExecutingAssembly().GetName().Name;

            // get the resource into a stream
            using (Stream str = Assembly.GetExecutingAssembly().GetManifestResourceStream(strNameSpace + wav))
            {
                string tempfile = System.IO.Path.GetTempFileName();
                tempFiles.AddFile(tempfile,false);
                var bStr = new Byte[str.Length];
                str.Read(bStr, 0, (int)str.Length);
                File.WriteAllBytes(tempfile, bStr);
                cachedWavs.Add(wav, tempfile);
                return tempfile;
            }
        }
    }
    return retVal;
}

It's coarse, but it works, and now I can move on to some cleanup with this bug behind me. The Back to Basics lesson for me is:

• Don't forget there is a Garbage Collector out there, doing just that.
• It's easy to forget all about it, but it's so important to know who has a finger on your memory when you're moving back and forth over the unmanaged/managed boundary.
• Edge cases are no fun.
• There are always edge cases, race conditions and deadlocks to be found, and I'm sure I've got more left to find! (notice my lack of surety around the lock() call in the comments?)
• Know your patterns for best practices or, better yet, know where to go to find the answers.
• Your software typically runs exactly as you wrote it.
• Even though this was a GC doing something I didn't expect, it was doing its job with the code I provided it. Given how I was using the byte array, it's very deterministic in its behavior.
• Know what's going wrong before you try to fix it.
• Once I understood the bug, I was able to reproduce the bug much more easily. "Flakies" are scary, but Bugs are just bugs. If I tried to fix it without understanding it, I'd just be programming by coincidence. (And I may still be! That's the rub.)

Have a nice day!

About Scott

Scott Hanselman is a former professor, former Chief Architect in finance, now speaker, consultant, father, diabetic, and Microsoft employee. He is a failed stand-up comic, a cornrower, and a book author.

About   Newsletter

Hosting By