EXC_BAD_ACCESS on Midi Read about webmidikit HOT 18 CLOSED

I ran into this issue a long time ago and it was hard to pin down. Thanks to the ancient implementation of CoreMIDI, you can get random exceptions in Swift.

• snip -

I'm still looking into it.

from webmidikit.

does it happen consistently?

from webmidikit.

I only caught this once, then once I caught it sending some junk bytes (these were SysEx messages).
Apart from that, no matter how hard I tried, I couldn't get this to happen again ¯_(ツ)_/¯

from webmidikit.

@orchetect do you want to create a PR?

from webmidikit.

I ran into this issue a long time ago and it was hard to pin down. Thanks to the ancient implementation of CoreMIDI, you can get random exceptions that are hard to trace if you just pass MIDIPacketList.packet directly into MIDIPacketNext.

You need to do this instead:

p = withUnsafePointer(to: &p) { MIDIPacketNext($0).pointee }

@orchetect Unfortunately I caught a new EXC_BAD_ACCESS today with this new line.
I'm thinking it might be due to some thread syncronization issues?

from webmidikit.

@raduvarga Yes, was a red herring. Turns out the unsafe pointer access wasn't the issue.

• snip -

from webmidikit.

This is crazy. I’m tempted to look at the disassembly.

from webmidikit.

Welp, back to square one. After further testing, the crashes persist.

I did however stumble on a possible solution in another repo.

Check out:

https://github.com/krevis/MIDIApps/blob/main/Frameworks/SnoizeMIDI/SMMIDIUtilities.m

I'm not sure if that fully resolves the crashes, but another Swift-based CoreMIDI library had to resort to using a C function to iterate packets in a packetlist.

from webmidikit.

I used to have some C code in the package a while back. Do you want to make a PR?

from webmidikit.

I have a stable fix in my own code based on that C method, along with some unsafe memory access on the Swift side.

It's in a couple production apps now so I will see if any issues/crashes are reported back.

from webmidikit.

Ok. What are your apps btw?

from webmidikit.

@orchetect Sounds good! How did you test the crashes?
In my case it usually happened when sending many long SysEx messages, but couldn't consistently produce a crash.

My approach (in my fork) in the meantime was to use a dispatch queue to solve some thread synchronization issues found in the library (found with the Thread Sanitizer in XCode), in case those were the culprit.
Not sure if that specifically helped, but I haven't seen any crashes or received crash reports since (also using it in production apps).

from webmidikit.

@orchetect @adamnemecek I found the issue!

Turns out MIDIPacketNext was called too many times and the last one was the one always causing the crash.
For example, if numPackets is 1, MIDIPacketNext is called 2 times.
So all we need is to avoid calling it for the last element.

I have a use case that was producing consistent crashes with the old code, now with the one below it works:

public func makeIterator() -> AnyIterator<Element> {
        var p: MIDIPacket = packet
        var i = (0..<numPackets).makeIterator()
        var index = 0
        
        return AnyIterator {
            defer {
                if (index < numPackets) {
                    p = withUnsafePointer(to: &p) { MIDIPacketNext($0).pointee }
                    index += 1
                }
            }
            
            return index < numPackets ? i.next().map { _ in .init(packet: &p) } : nil
        }
    }
}

from webmidikit.

How about this from AudioKit? https://github.com/Joachimholst95/Tunit2_0/blob/0a15fee2fdacdcc0c7d5f8c402559eb127622687/AudioKit/Common/MIDI/Packets/MIDIPacketList%2BSequenceType.swift

extension MIDIPacketList: Sequence {
    public typealias Element = MIDIPacket

    public var count: UInt32 {
        return self.numPackets
    }

    public func makeIterator() -> AnyIterator<Element> {
        var p: MIDIPacket = packet
        var idx: UInt32 = 0

        return AnyIterator {
            guard idx < self.numPackets else {
                return nil
            }
            defer {
                p = MIDIPacketNext(&p).pointee
                idx += 1
            }
            return p
        }
    }
}

from webmidikit.

Looks more elegant. In our case just needs a return .init(packet: &p) and we're sorted.

from webmidikit.

Want to submit a PR?

from webmidikit.

Just did.

from webmidikit.

Hopefully fixed by #30.

from webmidikit.