VST3 Client: Properly notify host about added/removed event loop FDs

modules/juce_audio_plugin_client/VST3/juce_VST3_Wrapper.cpp

@@ -70,9 +70,8 @@ JUCE_BEGIN_NO_SANITIZE ("vptr")
 #endif
 
 #if JUCE_LINUX || JUCE_BSD
+ #include "juce_events/native/juce_linux_EventLoopInternal.h"
  #include <unordered_map>
-
- std::vector<std::pair<int, std::function<void (int)>>> getFdReadCallbacks();
 #endif
 
 #if JUCE_MAC
@@ -103,14 +102,20 @@ using namespace Steinberg;
 //==============================================================================
 #if JUCE_LINUX || JUCE_BSD
 
-class EventHandler final  : public Steinberg::Linux::IEventHandler
+class EventHandler final  : public Steinberg::Linux::IEventHandler,
+                            private LinuxEventLoopInternal::Listener
 {
 public:
-    EventHandler() = default;
+    EventHandler()
+    {
+        LinuxEventLoopInternal::registerLinuxEventLoopListener (*this);
+    }
 
-    ~EventHandler()
+    ~EventHandler() override
     {
-        jassert (hostRunLoops.size() == 0);
+        jassert (hostRunLoops.empty());
+
+        LinuxEventLoopInternal::deregisterLinuxEventLoopListener (*this);
 
         if (! messageThread->isRunning())
             messageThread->start();
@@ -126,11 +131,7 @@ public:
     void PLUGIN_API onFDIsSet (Steinberg::Linux::FileDescriptor fd) override
     {
         updateCurrentMessageThread();
-
-        auto it = fdCallbackMap.find (fd);
-
-        if (it != fdCallbackMap.end())
-            it->second (fd);
+        LinuxEventLoopInternal::invokeEventLoopCallbackForFd (fd);
     }
 
     //==============================================================================
@@ -138,19 +139,7 @@ public:
     {
         if (auto* runLoop = getRunLoopFromFrame (plugFrame))
         {
-            if (hostRunLoops.contains (runLoop))
-                runLoop->unregisterEventHandler (this);
-
-            hostRunLoops.add (runLoop);
-
-            fdCallbackMap.clear();
-
-            for (auto& cb : getFdReadCallbacks())
-            {
-                fdCallbackMap[cb.first] = cb.second;
-                runLoop->registerEventHandler (this, cb.first);
-            }
-
+            refreshAttachedEventLoop ([this, runLoop] { hostRunLoops.insert (runLoop); });
             updateCurrentMessageThread();
         }
     }
@@ -158,67 +147,58 @@ public:
     void unregisterHandlerForFrame (IPlugFrame* plugFrame)
     {
         if (auto* runLoop = getRunLoopFromFrame (plugFrame))
-        {
-            hostRunLoops.remove (runLoop);
-
-            if (! hostRunLoops.contains (runLoop))
-                runLoop->unregisterEventHandler (this);
-        }
+            refreshAttachedEventLoop ([this, runLoop] { hostRunLoops.erase (runLoop); });
     }
 
 private:
-    //=============================================================================
-    class HostRunLoopInterfaces
+    //==============================================================================
+    /*  Connects all known FDs to a single host event loop instance. */
+    class AttachedEventLoop
     {
     public:
-        HostRunLoopInterfaces() = default;
+        AttachedEventLoop() = default;
 
-        void add (Steinberg::Linux::IRunLoop* runLoop)
+        AttachedEventLoop (Steinberg::Linux::IRunLoop* loopIn, Steinberg::Linux::IEventHandler* handlerIn)
+            : loop (loopIn), handler (handlerIn)
         {
-            if (auto* refCountedRunLoop = find (runLoop))
-            {
-                ++(refCountedRunLoop->refCount);
-                return;
-            }
-
-            runLoops.push_back ({ runLoop, 1 });
+            for (auto& fd : LinuxEventLoopInternal::getRegisteredFds())
+                loop->registerEventHandler (handler, fd);
         }
 
-        void remove (Steinberg::Linux::IRunLoop* runLoop)
+        AttachedEventLoop (AttachedEventLoop&& other) noexcept
         {
-            if (auto* refCountedRunLoop = find (runLoop))
-                if (--(refCountedRunLoop->refCount) == 0)
-                    runLoops.erase (std::find (runLoops.begin(), runLoops.end(), runLoop));
+            swap (other);
         }
 
-        size_t size() const noexcept                         { return runLoops.size(); }
-        bool contains (Steinberg::Linux::IRunLoop* runLoop)  { return find (runLoop) != nullptr; }
-
-    private:
-        struct RefCountedRunLoop
+        AttachedEventLoop& operator= (AttachedEventLoop&& other) noexcept
         {
-            Steinberg::Linux::IRunLoop* runLoop = nullptr;
-            int refCount = 0;
+            swap (other);
+            return *this;
+        }
 
-            bool operator== (const Steinberg::Linux::IRunLoop* other) const noexcept { return runLoop == other; }
-        };
+        AttachedEventLoop (const AttachedEventLoop&) = delete;
+        AttachedEventLoop& operator= (const AttachedEventLoop&) = delete;
 
-        RefCountedRunLoop* find (const Steinberg::Linux::IRunLoop* runLoop)
+        ~AttachedEventLoop()
         {
-            auto iter = std::find (runLoops.begin(), runLoops.end(), runLoop);
-
-            if (iter != runLoops.end())
-                return &(*iter);
+            if (loop == nullptr)
+                return;
 
-            return nullptr;
+            loop->unregisterEventHandler (handler);
         }
 
-        std::vector<RefCountedRunLoop> runLoops;
+    private:
+        void swap (AttachedEventLoop& other)
+        {
+            std::swap (other.loop, loop);
+            std::swap (other.handler, handler);
+        }
 
-        JUCE_DECLARE_NON_MOVEABLE (HostRunLoopInterfaces)
-        JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (HostRunLoopInterfaces)
+        Steinberg::Linux::IRunLoop* loop = nullptr;
+        Steinberg::Linux::IEventHandler* handler = nullptr;
     };
 
+    //==============================================================================
     static Steinberg::Linux::IRunLoop* getRunLoopFromFrame (IPlugFrame* plugFrame)
     {
         Steinberg::Linux::IRunLoop* runLoop = nullptr;
@@ -241,12 +221,44 @@ private:
         }
     }
 
+    void fdCallbacksChanged() override
+    {
+        // The set of active FDs has changed, so deregister from the current event loop and then
+        // re-register the current set of FDs.
+        refreshAttachedEventLoop ([]{});
+    }
+
+    /*  Deregisters from any attached event loop, updates the set of known event loops, and then
+        attaches all FDs to the first known event loop.
+
+        The same event loop instance is shared between all plugin instances. Every time an event
+        loop is added or removed, this function should be called to register all FDs with a
+        suitable event loop.
+
+        Note that there's no API to deregister a single FD for a given event loop. Instead, we must
+        deregister all FDs, and then register all known FDs again.
+    */
+    template <typename Callback>
+    void refreshAttachedEventLoop (Callback&& modifyKnownRunLoops)
+    {
+        // Deregister the old event loop.
+        // It's important to call the destructor from the old attached loop before calling the
+        // constructor of the new attached loop.
+        attachedEventLoop = AttachedEventLoop();
+
+        modifyKnownRunLoops();
+
+        // If we still know about an extant event loop, attach to it.
+        if (hostRunLoops.begin() != hostRunLoops.end())
+            attachedEventLoop = AttachedEventLoop (*hostRunLoops.begin(), this);
+    }
+
     SharedResourcePointer<MessageThread> messageThread;
 
     std::atomic<int> refCount { 1 };
 
-    HostRunLoopInterfaces hostRunLoops;
-    std::unordered_map<int, std::function<void (int)>> fdCallbackMap;
+    std::multiset<Steinberg::Linux::IRunLoop*> hostRunLoops;
+    AttachedEventLoop attachedEventLoop;
 
     //==============================================================================
     JUCE_DECLARE_NON_MOVEABLE (EventHandler)

modules/juce_events/juce_events.cpp

@@ -87,6 +87,7 @@
  #endif
 
 #elif JUCE_LINUX || JUCE_BSD
+ #include "native/juce_linux_EventLoopInternal.h"
  #include "native/juce_linux_Messaging.cpp"
 
 #elif JUCE_ANDROID

modules/juce_events/native/juce_linux_EventLoopInternal.h

@@ -0,0 +1,55 @@
+/*
+  ==============================================================================
+
+   This file is part of the JUCE library.
+   Copyright (c) 2022 - Raw Material Software Limited
+
+   JUCE is an open source library subject to commercial or open-source
+   licensing.
+
+   The code included in this file is provided under the terms of the ISC license
+   http://www.isc.org/downloads/software-support-policy/isc-license. Permission
+   To use, copy, modify, and/or distribute this software for any purpose with or
+   without fee is hereby granted provided that the above copyright notice and
+   this permission notice appear in all copies.
+
+   JUCE IS PROVIDED "AS IS" WITHOUT ANY WARRANTY, AND ALL WARRANTIES, WHETHER
+   EXPRESSED OR IMPLIED, INCLUDING MERCHANTABILITY AND FITNESS FOR PURPOSE, ARE
+   DISCLAIMED.
+
+  ==============================================================================
+*/
+
+namespace juce
+{
+
+struct LinuxEventLoopInternal
+{
+    /**
+        @internal
+
+        Receives notifications when a fd callback is added or removed.
+
+        This is useful for VST3 plugins that host other VST3 plugins. In this scenario, the 'inner'
+        plugin may want to register additional file descriptors on top of those registered by the
+        'outer' plugin. In order for this to work, the outer plugin must in turn pass this request
+        on to the host, to indicate that the outer plugin wants to receive FD callbacks for both the
+        inner and outer plugin.
+    */
+    struct Listener
+    {
+        virtual ~Listener() = default;
+        virtual void fdCallbacksChanged() = 0;
+    };
+
+    /** @internal */
+    static void registerLinuxEventLoopListener (Listener&);
+    /** @internal */
+    static void deregisterLinuxEventLoopListener (Listener&);
+    /** @internal */
+    static void invokeEventLoopCallbackForFd (int);
+    /** @internal */
+    static std::vector<int> getRegisteredFds();
+};
+
+} // namespace juce

modules/juce_events/native/juce_linux_Messaging.cpp

@@ -108,125 +108,168 @@ private:
 JUCE_IMPLEMENT_SINGLETON (InternalMessageQueue)
 
 //==============================================================================
+/*
+    Stores callbacks associated with file descriptors (FD).
+
+    The callback for a particular FD should be called whenever that file has data to read.
+
+    For standalone apps, the main thread will call poll to wait for new data on any FD, and then
+    call the associated callbacks for any FDs that changed.
+
+    For plugins, the host (generally) provides some kind of run loop mechanism instead.
+    - In VST2 plugins, the host should call effEditIdle at regular intervals, and plugins can
+      dispatch all pending events inside this callback. The host doesn't know about any of the
+      plugin's FDs, so it's possible there will be a bit of latency between an FD becoming ready,
+      and its associated callback being called.
+    - In VST3 plugins, it's possible to register each FD individually with the host. In this case,
+      the facilities in LinuxEventLoopInternal can be used to observe added/removed FD callbacks,
+      and the host can be notified whenever the set of FDs changes. The host will call onFDIsSet
+      whenever a particular FD has data ready. This call should be forwarded through to
+      InternalRunLoop::dispatchEvent.
+*/
 struct InternalRunLoop
 {
 public:
-    InternalRunLoop()
-    {
-        fdReadCallbacks.reserve (16);
-    }
+    InternalRunLoop() = default;
 
-    void registerFdCallback (int fd, std::function<void (int)>&& cb, short eventMask)
+    void registerFdCallback (int fd, std::function<void()>&& cb, short eventMask)
     {
-        const ScopedLock sl (lock);
-
-        if (shouldDeferModifyingReadCallbacks)
         {
-            deferredReadCallbackModifications.emplace_back ([this, fd, cb, eventMask]() mutable
-                                                            {
-                                                                registerFdCallback (fd, std::move (cb), eventMask);
-                                                            });
-            return;
+            const ScopedLock sl (lock);
+
+            callbacks.emplace (fd, std::make_shared<std::function<void()>> (std::move (cb)));
+
+            const auto iter = getPollfd (fd);
+
+            if (iter == pfds.end() || iter->fd != fd)
+                pfds.insert (iter, { fd, eventMask, 0 });
+            else
+                jassertfalse;
+
+            jassert (pfdsAreSorted());
         }
 
-        fdReadCallbacks.push_back ({ fd, std::move (cb) });
-        pfds.push_back ({ fd, eventMask, 0 });
+        listeners.call ([] (auto& l) { l.fdCallbacksChanged(); });
     }
 
     void unregisterFdCallback (int fd)
     {
-        const ScopedLock sl (lock);
-
-        if (shouldDeferModifyingReadCallbacks)
         {
-            deferredReadCallbackModifications.emplace_back ([this, fd] { unregisterFdCallback (fd); });
-            return;
-        }
+            const ScopedLock sl (lock);
 
-        {
-            auto removePredicate = [=] (const std::pair<int, std::function<void (int)>>& cb)  { return cb.first == fd; };
+            callbacks.erase (fd);
 
-            fdReadCallbacks.erase (std::remove_if (std::begin (fdReadCallbacks), std::end (fdReadCallbacks), removePredicate),
-                                   std::end (fdReadCallbacks));
-        }
+            const auto iter = getPollfd (fd);
 
-        {
-            auto removePredicate = [=] (const pollfd& pfd)  { return pfd.fd == fd; };
+            if (iter != pfds.end() && iter->fd == fd)
+                pfds.erase (iter);
+            else
+                jassertfalse;
 
-            pfds.erase (std::remove_if (std::begin (pfds), std::end (pfds), removePredicate),
-                        std::end (pfds));
+            jassert (pfdsAreSorted());
         }
+
+        listeners.call ([] (auto& l) { l.fdCallbacksChanged(); });
     }
 
     bool dispatchPendingEvents()
     {
-        const ScopedLock sl (lock);
+        callbackStorage.clear();
+        getFunctionsToCallThisTime (callbackStorage);
 
-        if (poll (&pfds.front(), static_cast<nfds_t> (pfds.size()), 0) == 0)
-            return false;
+        // CriticalSection should be available during the callback
+        for (auto& fn : callbackStorage)
+            (*fn)();
 
-        bool eventWasSent = false;
+        return ! callbackStorage.empty();
+    }
 
-        for (auto& pfd : pfds)
+    void dispatchEvent (int fd) const
+    {
+        const auto fn = [&]
         {
-            if (pfd.revents == 0)
-                continue;
+            const ScopedLock sl (lock);
+            const auto iter = callbacks.find (fd);
+            return iter != callbacks.end() ? iter->second : nullptr;
+        }();
+
+        // CriticalSection should be available during the callback
+        if (auto* callback = fn.get())
+            (*callback)();
+    }
+
+    bool sleepUntilNextEvent (int timeoutMs)
+    {
+        const ScopedLock sl (lock);
+        return poll (pfds.data(), static_cast<nfds_t> (pfds.size()), timeoutMs) != 0;
+    }
+
+    std::vector<int> getRegisteredFds()
+    {
+        const ScopedLock sl (lock);
+        std::vector<int> result;
+        result.reserve (callbacks.size());
+        std::transform (callbacks.begin(),
+                        callbacks.end(),
+                        std::back_inserter (result),
+                        [] (const auto& pair) { return pair.first; });
+        return result;
+    }
+
+    void addListener    (LinuxEventLoopInternal::Listener& listener)         { listeners.add    (&listener); }
+    void removeListener (LinuxEventLoopInternal::Listener& listener)         { listeners.remove (&listener); }
 
-            pfd.revents = 0;
+    //==============================================================================
+    JUCE_DECLARE_SINGLETON (InternalRunLoop, false)
+
+private:
+    using SharedCallback = std::shared_ptr<std::function<void()>>;
+
+    /*  Appends any functions that need to be called to the passed-in vector.
 
-            auto fd = pfd.fd;
+        We take a copy of each shared function so that the functions can be called without
+        locking or racing in the event that the function attempts to register/deregister a
+        new FD callback.
+    */
+    void getFunctionsToCallThisTime (std::vector<SharedCallback>& functions)
+    {
+        const ScopedLock sl (lock);
+
+        if (! sleepUntilNextEvent (0))
+            return;
 
-            for (auto& fdAndCallback : fdReadCallbacks)
+        for (auto& pfd : pfds)
+        {
+            if (std::exchange (pfd.revents, 0) != 0)
             {
-                if (fdAndCallback.first == fd)
-                {
-                    {
-                        ScopedValueSetter<bool> insideFdReadCallback (shouldDeferModifyingReadCallbacks, true);
-                        fdAndCallback.second (fd);
-                    }
-
-                    if (! deferredReadCallbackModifications.empty())
-                    {
-                        for (auto& deferredRegisterEvent : deferredReadCallbackModifications)
-                            deferredRegisterEvent();
-
-                        deferredReadCallbackModifications.clear();
-
-                        // elements may have been removed from the fdReadCallbacks/pfds array so we really need
-                        // to call poll again
-                        return true;
-                    }
-
-                    eventWasSent = true;
-                }
+                const auto iter = callbacks.find (pfd.fd);
+
+                if (iter != callbacks.end())
+                    functions.emplace_back (iter->second);
             }
         }
-
-        return eventWasSent;
     }
 
-    void sleepUntilNextEvent (int timeoutMs)
+    std::vector<pollfd>::iterator getPollfd (int fd)
     {
-        poll (&pfds.front(), static_cast<nfds_t> (pfds.size()), timeoutMs);
+        return std::lower_bound (pfds.begin(), pfds.end(), fd, [] (auto descriptor, auto toFind)
+        {
+            return descriptor.fd < toFind;
+        });
     }
 
-    std::vector<std::pair<int, std::function<void (int)>>> getFdReadCallbacks()
+    bool pfdsAreSorted() const
     {
-        const ScopedLock sl (lock);
-        return fdReadCallbacks;
+        return std::is_sorted (pfds.begin(), pfds.end(), [] (auto a, auto b) { return a.fd < b.fd; });
     }
 
-    //==============================================================================
-    JUCE_DECLARE_SINGLETON (InternalRunLoop, false)
-
-private:
     CriticalSection lock;
 
-    std::vector<std::pair<int, std::function<void (int)>>> fdReadCallbacks;
+    std::map<int, SharedCallback> callbacks;
+    std::vector<SharedCallback> callbackStorage;
     std::vector<pollfd> pfds;
 
-    bool shouldDeferModifyingReadCallbacks = false;
-    std::vector<std::function<void()>> deferredReadCallbackModifications;
+    ListenerList<LinuxEventLoopInternal::Listener> listeners;
 };
 
 JUCE_IMPLEMENT_SINGLETON (InternalRunLoop)
@@ -313,7 +356,7 @@ bool dispatchNextMessageOnSystemQueue (bool returnIfNoPendingMessages)
 void LinuxEventLoop::registerFdCallback (int fd, std::function<void (int)> readCallback, short eventMask)
 {
     if (auto* runLoop = InternalRunLoop::getInstanceWithoutCreating())
-        runLoop->registerFdCallback (fd, std::move (readCallback), eventMask);
+        runLoop->registerFdCallback (fd, [cb = std::move (readCallback), fd] { cb (fd); }, eventMask);
 }
 
 void LinuxEventLoop::unregisterFdCallback (int fd)
@@ -322,16 +365,31 @@ void LinuxEventLoop::unregisterFdCallback (int fd)
         runLoop->unregisterFdCallback (fd);
 }
 
-} // namespace juce
+//==============================================================================
+void LinuxEventLoopInternal::registerLinuxEventLoopListener (LinuxEventLoopInternal::Listener& listener)
+{
+    if (auto* runLoop = InternalRunLoop::getInstanceWithoutCreating())
+        runLoop->addListener (listener);
+}
+
+void LinuxEventLoopInternal::deregisterLinuxEventLoopListener (LinuxEventLoopInternal::Listener& listener)
+{
+    if (auto* runLoop = InternalRunLoop::getInstanceWithoutCreating())
+        runLoop->removeListener (listener);
+}
 
-JUCE_API std::vector<std::pair<int, std::function<void (int)>>> getFdReadCallbacks();
-JUCE_API std::vector<std::pair<int, std::function<void (int)>>> getFdReadCallbacks()
+void LinuxEventLoopInternal::invokeEventLoopCallbackForFd (int fd)
 {
-    using namespace juce;
+    if (auto* runLoop = InternalRunLoop::getInstanceWithoutCreating())
+        runLoop->dispatchEvent (fd);
+}
 
+std::vector<int> LinuxEventLoopInternal::getRegisteredFds()
+{
     if (auto* runLoop = InternalRunLoop::getInstanceWithoutCreating())
-        return runLoop->getFdReadCallbacks();
+        return runLoop->getRegisteredFds();
 
-    jassertfalse;
     return {};
 }
+
+} // namespace juce