plugins, translation units, and multiple instances of LeakedObjectDetector's LeakCounter singletons

[beOn]

I thought this was going to be a simple bug...

Here's the issue:

• Build the latest development branch (I'm on a mac using xcode, but this is probably a cross-platform issue).
• Run it.
• Throw a Spike Viewer in the signal chain.
• Open the Spike Viewer's tab, make sure it has focus, then press a key on your keyboard.
• You'll hit a breakpoint, and find this waiting for you in the debug console:

*** Dangling pointer deletion! Class: KeyPress
JUCE Assertion failure in juce_LeakedObjectDetector.h:71

Bummer! But Ok, I thought, let's fix it. And that's when my weekend plans went out the window.

Long story short, usually the LeakedObjectDetector class template uses one LeakCounter singleton per class that uses the template. So in this case, there's supposed to be one LeakCounter dedicated to counting constructions and deletions of KeyPress objects. But it turns out that singleton's don't behave as you might expect when combined with a plugin architecture - each plugin (and the main app) has its own compilation unit (translation unit), and, as things stand now, we actually get one LeakCounter singleton per class that uses the LeakedObjectDetector template per translation unit. That's crazy. It makes sense, but ugh, crazy. Let's take a closer look at the craziness, just to be sure.

First things first. The issue goes away if you comment out the entire body of SpikeDisplayCanvas::keyPressed. If you then un-comment only the line

KeyPress c = KeyPress::createFromDescription("c");

... the issue returns. So, simply instantiating then deallocating a KeyPress within this method causes a false dangling pointer alert. That's weird. But check out what happens if I modify getCounter to talk to me a bit (code borrowed and modified from here):

--- a/JuceLibraryCode/modules/juce_core/memory/juce_LeakedObjectDetector.h
+++ b/JuceLibraryCode/modules/juce_core/memory/juce_LeakedObjectDetector.h
@@ -107,6 +107,13 @@ private:
     static LeakCounter& getCounter() noexcept
     {
         static LeakCounter counter;
+ 
+        static int LeakCounterCounter; 
+ 
+        if (strcmp (getLeakedObjectClassName(), "KeyPress") == 0) { 
+            DBG (String::formatted ("%d: function pointer %p, counter object: %p, value: %d", LeakCounterCounter++, getCounter, &counter, counter.numObjects.get ())); 
+        } 
+ 
         return counter;
     }
 };

If I do that, then repeat the steps above, I get:

...(omitting output prior to my keypress)
791: function pointer 0x10019fac0, counter object: 0x100942578, value: 25
0: function pointer 0x107485ca0, counter object: 0x1074910e0, value: 0
*** Dangling pointer deletion! Class: KeyPress
JUCE Assertion failure in juce_LeakedObjectDetector.h:71

Different counter objects, different function pointers... different LeakCounterCounters... ugh. If I put a breakpoint on my DBG line and stop there during the offending call, then take a look at the stack to see which ~LeakedObjectDetector is being called, I get:

BasicSpikeDisplay`juce::LeakedObjectDetector<juce::KeyPress>::~LeakedObjectDetector:

... whereas if I do the same during a call to getCounter from any use of KeyPress within the main app, I get:

open-ephys`juce::LeakedObjectDetector<juce::KeyPress>::~LeakedObjectDetector:

You can get the same thing to happen in any one of the plugins, just by creating a KeyPress object. I haven't looked at other objects that use JUCE_LEAK_DETECTOR yet.

So, what to do? We could just set JUCE_CHECK_MEMORY_LEAKS to false for plugins, but that's unsatisfying in a couple ways: it doesn't address the multiple singletons issue, and kills memory leak checking for objects only ever used in a single plugin. It seems like someone with stronger linker-fu than I might have a helpful suggestion here.

(edit: originally included wrong line from stack trace)

[aacuevas]

Ok, I've tested it and it happens on Linux and OSX, but not on Windows (go figure...). Different linkers working in different ways, ugh...

I also feel it weird that we've not encountered this issue on any other class, just juce::KeyPress. We surely have other classes with leak detector instantiated both in the core GUI and in some plugins, so I don't know what makes the leak trigger in this case and not others, but I doubt this will be the only one.

JUCE is also full of templates, which are not ideal for plugins. For most of them I don't think it's an actual issue to have them exist in multiple units, as they just behave independently, but here it uses static members, which I believe are creating a big issue.

I doubt this will have an easy fix, so any ideas are welcome.

[beOn]

It might kinda make sense that it doesn’t happen on Windows… are you using the cdecl calling convention on everything? If so, perhaps the object is being cleaned up by the caller rather than the callee, and the caller to keyPressed just happens to be part of the main app.

On Sep 5, 2016, at 11:16 AM, Aarón Cuevas López notifications@github.com wrote:

Ok, I've tested it and it happens on Linux and OSX, but not on Windows (go figure...). Different linkers working in different ways, ugh...

I also feel it weird that we've not encountered this issue on any other class, just juce::KeyPress. We surely have other classes with leak detector instantiated both in the core GUI and in some plugins, so I don't know what makes the leak trigger in this case and not others, but I doubt this will be the only one.

JUCE is also full of templates, which are not ideal for plugins. For most of them I don't think it's an actual issue to have them exist in multiple units, as they just behave independently, but here it uses static members, which I believe are creating a big issue.

I doubt this will have an easy fix, so any ideas are welcome.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub #89 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AAC-RBp2G5ucTd-tvJpgCm8YsBe5a6bWks5qnEB5gaJpZM4J1JRj.

[cstawarz]

I'm pretty sure pull request #95 resolves this issue on macOS.

The issue seems to have been that Xcode was automatically adding -fvisibility-inlines-hidden to the compiler options for both debug and release builds. This flag is part of GCC's visibility controls (also supported by clang) and is a restricted version of its big brother -fvisibility=hidden. The important point is that it "causes all inlined class member functions to have hidden visibility".

In the case of the leak detector stuff, I think what happened is that the application binary and every plugin binary got its own private instance of getCounter (and presumably all the other inline methods in the definition of LeakedObjectDetector). Adding GCC_INLINES_ARE_PRIVATE_EXTERN = NO to debug builds prevents Xcode from using -fvisibility-inlines-hidden, which seems to resolve the issue.

I'd guess that the same basic issue is at play on Linux. Looking at the Makefile, I see that the compiler flags for both debug and release builds include -fvisibility=hidden, which implies -fvisibility-inlines-hidden. I presume the solution would be to remove that flag from debug builds, although I don't see where to set configuration-specific compiler flags for Linux builds in Projucer.

[beOn]

#95 did the trick. Thanks @cstawarz!

[alejoe91]

Hi,
I've had the same issue on Linux (Ubuntu16.10) and the keyPressed() issue is solved if I comment out the -fvisibility=hidden CPPFLAGS in the Makefile (1st out of 3 of them).

Unfortunately, if I compile without -fvisibility=hidden the RhythmFPGA plugin stops working and it gives segmentation fault whenever I try to put it in the chain.

The program stops here (okFrontPanelDLL,cpp, line 1328):

//------------------------------------------------------------------------
// Function calls - okCFrontPanel
//------------------------------------------------------------------------
okDLLEXPORT okFrontPanel_HANDLE DLL_ENTRY
okFrontPanel_Construct()
{
	if (_okFrontPanel_Construct)
		return((*_okFrontPanel_Construct)());

	return(NULL);
}

If I recompile with the -fvisibility=hidden CPPFLAG the RhtythmFPGA plugin works fine, but the keyPressed() problem reappears.
Any idea why this might happen and how I could solve it?

Thanks!