winmm teardown fixes - #243
Merged
Merged
Conversation
close_port() freed each MIDIHDR and its lpData as soon as midiInUnprepareHeader() returned, while the device was still open. For as long as it is, both the driver and midiInputCallback() - which requeues buffers through midiInAddBuffer() - can still hand them back to WinMM, so the driver ends up owning freed memory. That shows up much later as a heap-metadata failure inside RtlFreeHeap(), in an unrelated allocation. Buffers are now released only once midiInClose() has succeeded, and only once the callbacks in flight have finished with them. The callback registers itself through a counter which do_close() waits on, rather than taking a lock: the callback runs the user's message handler, and holding a lock across that would invert against whatever lock the thread closing the port already holds. The wait is bounded, and gives up by leaking the buffers rather than by hanging or by freeing them from under a callback. If midiInClose() fails, the handle and the buffers are kept so that a later close_port() - the destructor's, at the latest - can retry the whole teardown, and do_open() refuses to reopen a port whose device is still around. Along the way: - midiInClose()'s result was discarded in four places, so a device that refused to close was left open with a callback pointing at an object about to be destroyed. It is now checked and propagated. - inHandle was never initialized, and ~midi_in_winmm() calls close_port() unconditionally - bypassing the is_port_open() guard in midi_in.cpp - so any instance whose port was never opened passed garbage to midiInReset() and midiInClose(). - close_port() now returns before touching _mutex, which is not initialized when the constructor's InitializeCriticalSectionAndSpinCount() failed. - the sysex callback indexed sysexBuffer with a dwUser the driver hands back, unchecked. It now validates it against the buffer it claims to designate. - do_open()'s failure paths called midiInClose() with headers still prepared and queued, and left the buffers behind. - MIDIHDR was allocated with new MIDIHDR, leaving dwBytesRecorded and the reserved members indeterminate. The callback branches on dwBytesRecorded. - sysexBuffer.resize() is a no-op when the vector already has that size, so a second do_open() without an intervening close leaked the previous set. The unprepare/close retry loops are now bounded by a single deadline rather than by a per-call count: Sleep(1) lasts a timer tick, ~15.6ms unless something in the process raised the resolution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bytes_for_message() only knew about 0xF8 and 0xFE among the one-byte system messages, so 0xF6, 0xFA, 0xFB, 0xFC and 0xFF returned a length of zero. The callback then handed on_bytes() an empty span, which the input state machine drops - a sequencer following MIDI clock would see the ticks but never the transport commands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
observer_threaded and observer_manual each redeclared a `configuration` member which hid observer_winmm's and was never populated from the constructor arguments, so every use of `this->configuration` in the derived classes read a default-constructed one: - observer_threaded polled at the default 100ms whatever poll_period the caller asked for; - observer_manual called `this->configuration.manual_poll`, an empty std::function, throwing std::bad_function_call from its constructor - which is the only path make<observer_winmm>() takes when manual_poll is set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.