EXC_BAD_ACCESS on Midi Read

[raduvarga]

Line 94 from MidiPacketList.swift crashes with EXC_BAD_ACCESS.

p = MIDIPacketNext(&p).pointee

Any idea how to safeguard this line?

[adamnemecek]

does it happen consistently?

[raduvarga]

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 ¯_(ツ)_/¯

[orchetect]

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.

[adamnemecek]

@orchetect do you want to create a PR?

[raduvarga]

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?

[orchetect]

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

• snip -

[adamnemecek]

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

[orchetect]

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.

[adamnemecek]

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

[orchetect]

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.

[adamnemecek]

Ok. What are your apps btw?

[raduvarga]

@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).

[raduvarga]

@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
        }
    }
}

[adamnemecek]

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
        }
    }
}

[raduvarga]

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

[adamnemecek]

Want to submit a PR?

[raduvarga]

Just did.

[adamnemecek]

Hopefully fixed by #30.