Memory Leaks in message-handling changes (FOUND)


#1

The commit with hash 31209dadfceed8283c09a83d38eaa2fa13a7e3af and comment “Simplified some message-handling code” causes memory leaks in my application. Rolling back to the commit just before (“Fix for IPV6 sockets”) eliminates the leak.

The only thing I’m using is an AsyncUpdater. I’m not sure why the leak is happening. I looked at the diffs in the commit and nothing is jumping out at me, although I did not dig deep.

What I did notice is callFunctionOnMessageThread() with a const ReferenceCountedObjectPtr automatic variable. Perhaps something is up with that?

This happens under Windows / MSVC 2010.


#2

Hmm… Can’t see anything wrong there, and am pretty sure the use of the ref-counted pointer in callFunctionOnMessageThread is legit.

Exactly what class is getting leaked?


#3

I’m not sure at the moment but they are a handful of 16-byte objects


#4

mee too! 2 x 16 bytes ( i do nothing special )


#5

Do your leaks disappear when you roll back Juce to the commit that I mentioned in the OP?


#6

If it really is callFunctionOnMessageThread that’s causing the problem, then you can check by hacking the AsyncFunctionCallback class in juce_MessageManager.cpp to give it a JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR. (Seems to be fine when I try it on OSX).


#7

I will give that a go but first I will try to get rid of this memory corruption with the non-debug C Runtime and hope that makes the other problems vanish. Although it is suspicious that the leak appears on a specific Juce commit.


#8

Well, I did change the implementation so that a message gets allocated, when before I don’t think it allocated anything. Can’t see anything wrong with the implementation though.


#9

It seems even after fixing the AppConfig.h problem that this leak still exists…


#10

I further isolated this to these three files in Commit 31209dadfceed8283c09a83d38eaa2fa13a7e3af

modules\juce_events\messages\juce_MessageManager.cpp
modules\juce_events\messages\juce_MessageManager.h
modules\juce_events\native\juce_win32_Messaging.cpp

Also, I added leak detection to both CallbackMessage and AsyncUpdaterMessage and it is definitely both of these that are getting leaked (there are a few more CallbackMessage leaked than AsyncUpdaterMessage):

*** Leaked objects detected: 12 instance(s) of class AsyncUpdaterMessage
AppDebugWin32.exe has triggered a breakpoint
*** Leaked objects detected: 13 instance(s) of class CallbackMessage
AppDebugWin32.exe has triggered a breakpoint
The thread 'Deleter' (0x13ac) has exited with code 0 (0x0).
The thread 'Once Per Second' (0x112c) has exited with code 0 (0x0).
Detected memory leaks!
Dumping objects ->
{388425} normal block at 0x00E85CA0, 24 bytes long.
 Data: <D               > 44 F3 B0 01 01 00 00 00 00 00 00 00 CD CD CD CD 
{13603} normal block at 0x0543EEE0, 12 bytes long.
 Data: <h           > 68 0E AF 01 01 00 00 00 00 00 00 00 
{4697} normal block at 0x00F230B8, 28 bytes long.
 Data: <                > F8 F0 AF 01 01 00 00 00 00 00 00 00 CD CD CD CD 
{4610} normal block at 0x00F21DF0, 28 bytes long.
 Data: <                > F8 F0 AF 01 01 00 00 00 00 00 00 00 CD CD CD CD 
{4579} normal block at 0x00F1D838, 28 bytes long.
 Data: <                > F8 F0 AF 01 01 00 00 00 00 00 00 00 CD CD CD CD 
{4550} normal block at 0x00F1D1F8, 28 bytes long.
 Data: <                > F8 F0 AF 01 01 00 00 00 00 00 00 00 CD CD CD CD 
{4521} normal block at 0x00F1CBA8, 28 bytes long.
 Data: <                > F8 F0 AF 01 01 00 00 00 00 00 00 00 CD CD CD CD 
{4492} normal block at 0x00F1C568, 28 bytes long.
 Data: <                > F8 F0 AF 01 01 00 00 00 00 00 00 00 CD CD CD CD 
{4462} normal block at 0x00F1BEC8, 28 bytes long.
 Data: <                > F8 F0 AF 01 01 00 00 00 00 00 00 00 CD CD CD CD 
{2108} normal block at 0x00EE4C08, 28 bytes long.
 Data: <                > F8 F0 AF 01 01 00 00 00 00 00 00 00 CD CD CD CD 
{1244} normal block at 0x00E9C848, 28 bytes long.
 Data: <                > F8 F0 AF 01 01 00 00 00 00 00 00 00 CD CD CD CD 
{1234} normal block at 0x00E9ADA8, 28 bytes long.
 Data: <                > F8 F0 AF 01 01 00 00 00 00 00 00 00 CD CD CD CD 
{1191} normal block at 0x00E9B618, 28 bytes long.
 Data: <                > F8 F0 AF 01 01 00 00 00 00 00 00 00 CD CD CD CD 
{1187} normal block at 0x00E9AFF8, 28 bytes long.
 Data: <                > F8 F0 AF 01 01 00 00 00 00 00 00 00 CD CD CD CD 
Object dump complete.

#11

What if there are pending CallbackMessage objects in the system message queue when we exit? Where does the system queue get drained?

It might be worthwhile to use a single system message to signal the presence of one or more Message objects, and maintain the queue separately instead of having 1 system message for each juce::Message. On Windows at least (?)


#12

!
FOUND IT!

The new version of MessageManager never sets quitMessagePosted = true. Adding the line fixes the problem:

void MessageManager::stopDispatchLoop()
{
  (new QuitMessage())->post();
  quitMessagePosted = true;
}

I believe the reason it leaks is because other threads are calling AsyncUpdater::triggerAsyncUpdate() which leads to MessageManager::postMessageToQueue():

void MessageManager::postMessageToQueue (Message* const message)
{
    if (quitMessagePosted || ! postMessageToSystemQueue (message))
        Message::Ptr deleter (message); // (this will delete messages that were just created with a 0 ref count)
}

Since quitMessagePosted was false even after MessageManager::stopDispatchLoop() is called, Message-derived objects get put into the system queue from which they are never processed.

This problem will manifest itself for anyone who calls AsyncUpdater::triggerAsyncUpdate() from threads other than the message thread, after the QuitMessage is posted, in the latest tip.


#13

Awesome bug-sniffing, Vinnie, thanks so much!

(I’m slightly embarrassed about not spotting that myself though…!)