From 69b630a2c0771a397c265b8aae38ce035f974458 Mon Sep 17 00:00:00 2001 From: reuk Date: Mon, 22 Feb 2021 19:36:35 +0000 Subject: [PATCH] CoreMidi: Use RAII to avoid potential leaks of MIDI ports/endpoints --- .../native/juce_mac_CoreMidi.mm | 104 ++++++++++++++---- 1 file changed, 80 insertions(+), 24 deletions(-) diff --git a/modules/juce_audio_devices/native/juce_mac_CoreMidi.mm b/modules/juce_audio_devices/native/juce_mac_CoreMidi.mm index 4c24bcc41b..49c8a60e5d 100644 --- a/modules/juce_audio_devices/native/juce_mac_CoreMidi.mm +++ b/modules/juce_audio_devices/native/juce_mac_CoreMidi.mm @@ -313,36 +313,89 @@ namespace CoreMidiHelpers using SenderToUse = Sender; + template + class ScopedMidiResource + { + public: + ScopedMidiResource() = default; + + explicit ScopedMidiResource (Resource r) : contents (r, {}) {} + + ~ScopedMidiResource() noexcept + { + auto ref = std::get<0> (contents); + + if (ref != 0) + std::get<1> (contents) (ref); + } + + ScopedMidiResource (const ScopedMidiResource& other) = delete; + ScopedMidiResource& operator= (const ScopedMidiResource& other) = delete; + + ScopedMidiResource (ScopedMidiResource&& other) noexcept { swap (other); } + + ScopedMidiResource& operator= (ScopedMidiResource&& other) noexcept + { + swap (other); + return *this; + } + + void swap (ScopedMidiResource& other) noexcept { std::swap (other.contents, contents); } + + Resource operator*() const noexcept { return std::get<0> (contents); } + + Resource release() noexcept + { + auto old = std::get<0> (contents); + std::get<0> (contents) = 0; + return old; + } + + private: + std::tuple contents { {}, {} }; + }; + + struct PortRefDestructor + { + void operator() (MIDIPortRef p) const noexcept { MIDIPortDispose (p); } + }; + + using ScopedPortRef = ScopedMidiResource; + + struct EndpointRefDestructor + { + void operator() (MIDIEndpointRef p) const noexcept { MIDIEndpointDispose (p); } + }; + + using ScopedEndpointRef = ScopedMidiResource; + //============================================================================== class MidiPortAndEndpoint { public: - MidiPortAndEndpoint (MIDIPortRef p, MIDIEndpointRef ep) noexcept - : port (p), endpoint (ep), sender (ep) + MidiPortAndEndpoint (ScopedPortRef p, ScopedEndpointRef ep) noexcept + : port (std::move (p)), endpoint (std::move (ep)), sender (*endpoint) {} ~MidiPortAndEndpoint() noexcept { - if (port != 0) - MIDIPortDispose (port); - - // if port == nullptr, it means we created the endpoint, so it's safe to delete it - if (port == 0 && endpoint != 0) - MIDIEndpointDispose (endpoint); + // if port != 0, it means we didn't create the endpoint, so it's not safe to delete it + if (*port != 0) + endpoint.release(); } void send (const MidiMessage& m) { - sender.send (port, endpoint, m); + sender.send (*port, *endpoint, m); } void send (ump::Iterator b, ump::Iterator e) { - sender.send (port, endpoint, b, e); + sender.send (*port, *endpoint, b, e); } - bool canStop() const noexcept { return port != 0; } - void stop() const { CHECK_ERROR (MIDIPortDisconnectSource (port, endpoint)); } + bool canStop() const noexcept { return *port != 0; } + void stop() const { CHECK_ERROR (MIDIPortDisconnectSource (*port, *endpoint)); } ump::MidiProtocol getProtocol() const noexcept { @@ -350,8 +403,8 @@ namespace CoreMidiHelpers } private: - MIDIPortRef port; - MIDIEndpointRef endpoint; + ScopedPortRef port; + ScopedEndpointRef endpoint; SenderToUse sender; }; @@ -1017,13 +1070,12 @@ public: if (! CHECK_ERROR (CreatorFunctionsToUse::createInputPort (protocol, client, cfName.cfString, input->internal.get(), &port))) continue; - if (! CHECK_ERROR (MIDIPortConnectSource (port, endpoint, nullptr))) - { - CHECK_ERROR (MIDIPortDispose (port)); - continue; - } + ScopedPortRef scopedPort { port }; - input->internal->portAndEndpoint = std::make_unique (port, endpoint); + if (! CHECK_ERROR (MIDIPortConnectSource (*scopedPort, endpoint, nullptr))) + continue; + + input->internal->portAndEndpoint = std::make_unique (std::move (scopedPort), ScopedEndpointRef { endpoint }); return input; } } @@ -1049,6 +1101,7 @@ public: ScopedCFString name (deviceName); auto err = CreatorFunctionsToUse::createDestination (protocol, client, name.cfString, input->internal.get(), &endpoint); + ScopedEndpointRef scopedEndpoint { endpoint }; #if JUCE_IOS if (err == kMIDINotPermitted) @@ -1066,7 +1119,7 @@ public: if (! CHECK_ERROR (MIDIObjectSetIntegerProperty (endpoint, kMIDIPropertyUniqueID, (SInt32) deviceIdentifier))) return {}; - input->internal->portAndEndpoint = std::make_unique ((MIDIPortRef) 0, endpoint); + input->internal->portAndEndpoint = std::make_unique (ScopedPortRef{}, std::move (scopedEndpoint)); return input; } } @@ -1185,8 +1238,10 @@ std::unique_ptr MidiOutput::openDevice (const String& deviceIdentifi if (! CHECK_ERROR (MIDIOutputPortCreate (client, cfName.cfString, &port))) continue; + ScopedPortRef scopedPort { port }; + auto midiOutput = rawToUniquePtr (new MidiOutput (endpointInfo.name, endpointInfo.identifier)); - midiOutput->internal = std::make_unique (port, endpoint); + midiOutput->internal = std::make_unique (std::move (scopedPort), ScopedEndpointRef { endpoint }); return midiOutput; } @@ -1206,6 +1261,7 @@ std::unique_ptr MidiOutput::createNewDevice (const String& deviceNam ScopedCFString name (deviceName); auto err = CreatorFunctionsToUse::createSource (ump::PacketProtocol::MIDI_1_0, client, name.cfString, &endpoint); + ScopedEndpointRef scopedEndpoint { endpoint }; #if JUCE_IOS if (err == kMIDINotPermitted) @@ -1222,11 +1278,11 @@ std::unique_ptr MidiOutput::createNewDevice (const String& deviceNam auto deviceIdentifier = createUniqueIDForMidiPort (deviceName, false); - if (! CHECK_ERROR (MIDIObjectSetIntegerProperty (endpoint, kMIDIPropertyUniqueID, (SInt32) deviceIdentifier))) + if (! CHECK_ERROR (MIDIObjectSetIntegerProperty (*scopedEndpoint, kMIDIPropertyUniqueID, (SInt32) deviceIdentifier))) return {}; auto midiOutput = rawToUniquePtr (new MidiOutput (deviceName, String (deviceIdentifier))); - midiOutput->internal = std::make_unique ((UInt32) 0, endpoint); + midiOutput->internal = std::make_unique (ScopedPortRef{}, std::move (scopedEndpoint)); return midiOutput; }