MIDI devices crashing on Windows

Windows
1

I’m having trouble with MIDI devices crashing my app on Windows, haven’t figured out why yet.

I added the following to MidiOutHandle:

        MidiOutHandle() { DBG(String::formatted("Han Created %p", this)); }
        ~MidiOutHandle() { DBG(String::formatted("Han Deleted %p", this)); }

Then in my debug log I see:

Startup:
Han Created 0000000004123D50
Han Created 0000000004123BD0
Han Deleted 0000000004123D50

Shutdown:
Han Deleted 0000000004123BD0
Han Deleted 0000000004123D50
*** Dangling pointer deletion! Class: MidiOutHandle

For some reason when the second MIDI input is opened, it is deleting the MidiOutHandle for the first input. Then they both get deleted again on shutdown.

Still digging into it.

2

Ok, found the crash:

First there is this loop, which looks for a MidiOutputHandle and if there is assigns it to han (which is a ScopedPointer) and increases the reference count.

        for (int i = parent.activeOutputHandles.size(); --i >= 0;)
        {
            han = parent.activeOutputHandles.getUnchecked (i);

            if (han->deviceId == deviceId)
            {
                han->refCount++;

                return;
            }
        }

However, if it doesn’t find a han with a matching deviceId, it doesn’t clear the ScopedPointer. And the next loop creates a new MidiOutputHandle and assigns it to han, which causes the previous MidiOutputHandle to get deleted.

        for (int i = 4; --i >= 0;)
        {
            HMIDIOUT h = 0;
            MMRESULT res = midiOutOpen (&h, deviceId, 0, 0, CALLBACK_NULL);

            if (res == MMSYSERR_NOERROR)
            {
                han = new MidiOutHandle();
                han->deviceId = deviceId;
                han->refCount = 1;
                han->handle = h;
                parent.activeOutputHandles.add (han);

                return;
            }
            else if (res == MMSYSERR_ALLOCATED)
            {
                Sleep (100);
            }
            else
            {
                break;
            }
        }
3

Here is my fix:

        for (int i = parent.activeOutputHandles.size(); --i >= 0;)
        {
            auto* activeHandle = parent.activeOutputHandles.getUnchecked (i);

            if (activeHandle->deviceId == deviceId)
            {
                activeHandle->refCount++;
                han = activeHandle;

                return;
            }
        }
4

Thank you for reporting!

A fix will appear on the develop branch shortly.

1 Like
5

This isn’t fixed correctly: https://github.com/julianstorer/JUCE/commit/9f06fabe105b91b952a3c679b810a0dac45d6c20

if (han->deviceId == deviceId)

should be

if (activeHandle->deviceId == deviceId)

6

Thanks again - this is now properly fixed!

7

I believe there is still an issue with this. If multiple references to a single MidiOutHandle exist, each will be deleted when the referencing WindowsOutputWrapper is deleted (han is a ScopedPointer). This may address deletion of dangling pointers:

    ~WindowsOutputWrapper()
    {
        if (parent.activeOutputHandles.contains (han.get()) && --(han->refCount) == 0)
        {
            midiOutClose (han->handle);
            parent.activeOutputHandles.removeFirstMatchingValue (han.get());
        }
        else
        {
            han.release ();
        }
    }
1 Like
8

Here’s a simple repro scenario:

if (MidiOutput::getDevices ().size ())
{
    ScopedPointer <MidiOutput> a = MidiOutput::openDevice (0);
    ScopedPointer <MidiOutput> b = MidiOutput::openDevice (0);
}
2 Likes