BlockingMessage accessing MessageManager::Lock that went out of scope

General JUCE discussion
1

I believe that there is an issue in the interaction between MessageManager::Lock::BlockingMessage and the MessageManager::Lock.

I can’t see that there is any guarantee that the MessageManagerLock did not go out of scope after MessageManager::Lock::abort() calls abortWait.set (1). When abortWait is 1 we will return true from MessageManager::Lock::tryAquire and the MessageManagerLock will advance in it constructor. The code that is protected by the MessageManagerLock can run and then the MessageManagerLock will go out of scope and call MessageManager::Lock::exit. In exit there is nothing that wait for the message thread, so the BlockingMessage::messageCallback might still be executing. Maybe we did not advance at all, and the next thing that happens is that lockedEvent.signal() is called, which is now being called on an object that no longer exists.

This seems to fix the issue:

@@ -386,6 +386,10 @@ void MessageManager::Lock::exit() const noexcept

         if (blockingMessage != nullptr)
         {
+            {
+              ScopedLock lock(blockingMessage->ownerCriticalSection);
+              blockingMessage->owner.set(nullptr);
+            }
             blockingMessage->releaseEvent.signal();
             blockingMessage = nullptr;
         }

Since blockingMessage is held in a ReferenceCountedObjectPtr<BlockingMessage> it should be fine to access it (it is already being done before the change), and when we hold the lock we can be sure that the BlockingMessage is not accessing the pointer to the MessageManager::Lock, i.e it is still not in any of the messageCallback s involved. If we do blockingMessage->owner.set(nullptr) then we can be sure that the messageCallback will not be called.

Something similar is already being done if we did not get the lock at the end of BlockingMessage::tryAquire.

I have seen this problem when we ran a vst3 plugin with pluginval, and it was easily reproducible. With the above fix the problem goes away.

*** FAILED: VALIDATION CRASHED

0   pluginval                           0x000000010baab75e _ZN4juce11SystemStats17getStackBacktraceEv + 94
1   pluginval                           0x000000010b571004 _ZN12_GLOBAL__N_119getCrashLogContentsEv + 36
2   pluginval                           0x000000010b570e65 _ZN12_GLOBAL__N_111handleCrashEPv + 21
3   pluginval                           0x000000010baab9f8 _ZN4juceL11handleCrashEi + 24
4   libsystem_platform.dylib            0x00007ff807e9bdfd _sigtramp + 29
5   ???                                 0x00007ff7b49ab7d8 0x0 + 140701863688152
6   libsystem_pthread.dylib             0x00007ff807e8243a _pthread_cond_check_init_slow + 207
7   libsystem_pthread.dylib             0x00007ff807e820a0 pthread_cond_broadcast + 94
8   libc++.1.dylib                      0x00007ff807de4ce5 _ZNSt3__118condition_variable10notify_allEv + 9
9   pluginval                           0x000000010babab80 _ZNK4juce13WaitableEvent6signalEv + 64
10  pluginval                           0x000000010bb853c2 _ZNK4juce14MessageManager4Lock5abortEv + 50
11  pluginval                           0x000000010bb8537e _ZNK4juce14MessageManager4Lock15messageCallbackEv + 46
12  pluginval                           0x000000010bb8e876 _ZN4juce14MessageManager4Lock15BlockingMessage15messageCallbackEv + 70
13  pluginval                           0x000000010bb99813 _ZN4juce12MessageQueue18deliverNextMessageEv + 99
14  pluginval                           0x000000010bb99766 _ZN4juce12MessageQueue15runLoopCallbackEv + 54
15  pluginval                           0x000000010bb99465 _ZN4juce12MessageQueue21runLoopSourceCallbackEPv + 21
16  CoreFoundation                      0x00007ff807f4d19b __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
17  CoreFoundation                      0x00007ff807f4d103 __CFRunLoopDoSource0 + 180
18  CoreFoundation                      0x00007ff807f4ce7d __CFRunLoopDoSources0 + 242
19  CoreFoundation                      0x00007ff807f4b898 __CFRunLoopRun + 892
20  CoreFoundation                      0x00007ff807f4ae5c CFRunLoopRunSpecific + 562
21  HIToolbox                           0x00007ff810c9d5e6 RunCurrentEventLoopInMode + 292
22  HIToolbox                           0x00007ff810c9d34a ReceiveNextEventCommon + 594
23  HIToolbox                           0x00007ff810c9d0e5 _BlockUntilNextEventMatchingListInModeWithFilter + 70
24  AppKit                              0x00007ff80a9861dd _DPSNextEvent + 927
25  AppKit                              0x00007ff80a98489a -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1394
26  AppKit                              0x00007ff80a976f49 -[NSApplication run] + 586
27  pluginval                           0x000000010bb830a9 _ZN4juce14MessageManager15runDispatchLoopEv + 153
28  pluginval                           0x000000010bb82f8f _ZN4juce19JUCEApplicationBase4mainEv + 399
29  pluginval                           0x000000010bb82d9c _ZN4juce19JUCEApplicationBase4mainEiPPKc + 60
30  pluginval                           0x000000010b573753 main + 51
31  dyld                                0x000000011af0f52e start + 462
2

Friendly bump :slight_smile:

3

Has there been any input of the JUCE devs on this topic?

4

@reuk I saw that you did some changes to MessageManager::Lock::exit earlier this year, so that when I now update to a newer version of juce then I can’t cherry-pick the commit that can be seen in the PR that is mentioned above.
Was your commit a result of the problem that I described above?

5

I believe that this issue should be fixed in 7.0.8, but I’d recommend testing it to make sure.

The most recent changes to the MessageManager Lock were made to address these issues:

1 Like