Help with crash report

I couldn’t locate the specific article mentioned in the SC book (the model implemented in SCLang), but I’ve compiled other papers by the same authors written around the same period:

Ok, little report on this, after I changed the dictionary copy thing I haven’t had any crashes anymore, and I did use WFSCollider intensively. I’d be interested to know under what circumstances RoundButton causes a crash, but it could indeed be that if you make an action in RoundButton (or in fact a UserView) that makes it remove itself (for example by closing the window) this sometimes causes a crash. I’ve put .defers everywhere where this happens in WFSCollider and had no crashes since (that is a while ago, the old scapp had a similar issue). From what I noticed copying QObjects is tricky, it can and will cause SC to crash at some point. I also had it with Image, it was copied in an undo state and that makes the interpreter crash after a while, especially when trying to draw it again. It is doable to prevent this in your code (in this case I made a global var containing images linked to symbols so it copies only the reference and not the Image itself). And for the QPalettes, those I store in a function that is called only when the QPalette is needed, Functions garbage-collect just fine. I’m not even sure if it is the garbage collector causing issues here or maybe pointers to objects in Qt.

One thing I did notice is that Menu’s and MenuActions don’t get garbage collected unless you call .destroy on them explicitly. Also, calling .destroy on a Menu doesn’t destroy the MenuActions in it. I made a .deepDestroy method for this in Unit-Lib, that works as advertised and stops the build-up of uncollected objects in the QObject.heap (I’ve had cases where there were >10.000 MenuActions in there…). This is clearly a bug in sc; the docs say that Menu’s get garbage-collected after use, and in several places in the code stray Menu objects are created (for example when calling Menu.front - I made a .uFront method in Unit-Lib that prevents this), and these stay in the heap forever. I think that was also one cause of crashing I experienced.

@jordan and @muellmusik are working to improve the garbage collector and find the causes of some of those bugs. Jordan just came up with a fix and is in the testing phase. At some point, you should be in touch with them to figure out more about it as well.

Maybe we will need to write some specific tests to find out more.

yes, it needs investigation for sure.

If you could put together a little bit of code that shows this, without any quarks, just vanilla sc, and submit a GitHub issue, I’ll take a look. If it is as you describe, this needs to get fixed.

Seconded. Also FWIW @Wouter_Snoei I’ve seen crashes when a RoundButton is pressed, without removing itself or closing anything. This is definitely a somewhat recent change as it’s in a GUI we’ve been using for years. Again any simple reproducer (even if not 100% occurring) would be super useful!

Just saw this thread. For what it’s worth, this problem with MenuAction was identified after I introduced them some years ago. After digging in quite deeply, I discovered that - at least by my observations, finalizers were not working correctly in a pretty fundamental way. I saw them being called inconsistantly / not at all (though never this particular case of a finalizer being called on something that seems already dead?).

This isn’t SO surprising since finalizers are barely used in the language - mainly for some relatively uncommon QT objects like QPalette - there are no unit tests for these, and it’s likely that the finalizer implementation is just whatever JmC wrote on a first pass in the original sclang runtime and probably barely touched after that (no shade to JmC here, just saying that a barely used feature that has no tests guaranteeing it’s behavior is a likely candidate for subtle bugs). Probably, this has never worked 100% correctly. Here’s some more backstory: Finalizers are not run consistently / regularly, resulting in memory leak behavior. · Issue #4288 · supercollider/supercollider · GitHub

At a glance, this sticks out: VM Region Info: 0x47fe72a8000 is not in any region.
This is a somewhat plausible address for an object (it’s usually more obvious when you have a pointer that’s actually just random data), but is definitely not pointing to anything valid. If you’re explicitly using Palettes in your code, this is actually easy to investigate - the bad pointer in question is the first field of your QPalette, I think you can literally print it with QPalette().slotAt(0).postln - if you’re using QPalette directly, print them out and see which one is crashing later.

I’m very suspicious of deepCopy here - I can almost 100% guarantee that calling deepCopy on a QPalette is invalid, since it counts as a new object to sclang but is still just a pointer to the same QPalette underneath - I don’t really see a reason why it’s finalizer wouldn’t get called twice in this case, which is obviously a time bomb. It’s worth checking to see if deepCopy has any protections against copying RawPointer objects - if it does not, this probably should be added (this could be done easily enough on the language side to test whether it fixes the crash).

1 Like

More context:

MenuActions are somewhat unique in their ownership - a menu does NOT own a MenuAction, these are intended in QT to be global, application-wide objects that are created once and stored. But, SuperCollider supports - for convenience obviously - a syntax that looks like Menu(MenuAction("foo")). With this code, the action is immediately lost to sclang, which means that some time in the near future the underlying QT object would (in theory) be deleted, removing it from the Menu. This would obviously make the menu system just… not work. The fact that finalizers are not called avoids this problem, but since it means we are not REALLY doing lifetime management on Actions, they leak. This is a problem for code that is creating tons and tons of ephemeral Actions in a loop, which is ofc SUPPORTED by QT but not really designed that way,

When I originally looked at the bug, I sketched a solution that looked like this:

  • Create a new QObject-derived class that has a QSharedPointer to the action (e.g. class QcActionReference { QSharedPointer<QcAction> actionRef; })
  • When you add an Action to a menu, create a new QcActionReferece and parent it to the menu you’re assigning the action. This means the Action will continue to live as long as the Menu, at least. auto ref = new QcActionReference{action}; ref.setParent(menu);
  • ALSO create a QcActionReference that the Action itself owns - this represents ownership by sclang. This can be an object tracked in sclang or hidden insize of QcAction.
  • Create a finalizer for QcAction that deletes it’s QcActionReference, effectively saying “sclang no longer needs this”
  • When sclang has finalized the QcAction, and the last menu that owns a QcActionReference dies, this QcAction will be destroyed.

I could never really implement this solution because - as I discovered - finalizers never really got called, so it was basically untestable / wouldn’t have done much anyway.

The pragmatic solution is to stick to usage that follows QT’s semantics: create a limited set of actions and re-use them, rather than re-creating actions again and again. By definition this means you won’t be leaking, since it avoids cases where a semantically identical Action is created again and again N times in code. This sucks as a constraint, but until the finalizer things can be ironed out I’m not sure what else there is to do?

I wish I could spend more time looking into this finalizer issue, but alas i’ve got too much on my plate right now. But if anyone want to dig into it, some pointers:

  • try the repro cases I mentioned in the bug (e.g. creating thousands of unused QObject-based objects, like Download or QPalette)
  • Look at all existing finalizers (you can see them installed with InstallFinalizer) - see which are being called and which not, maybe there’s a pattern
  • Look closely at ScanFinalizers to see when it is actually calling Finalize on objects (if i remember this 5-years-ago issue, the finalizers were almost never being called in spite of many finalizable object seemingly being collected)
  • Add asserts to the code to check assumptions - e.g. try settings a magic number value instead of Nil when Finalize does this: SetNil(obj->slots + 0);, and then assert later when you expect the finalizer should have been called (e.g. when the object memory is being collected).
  • During a GC scan, you are visiting every reachable object so you SHOULD be able to count every on that has a finalizer? |
2 Likes

@muellmusik about the RoundButton crash, I just pushed a possible fix for wslib. Since 25 days ago it was checking the current QtGUI palette to get the correct overlay color when disabled. Reading what has been said about Qt objects in this thread this could very well have been the culprit (even though the function isn’t actually called unless the button is disabled). I’ve changed it to a customizable color, so that in Unit-Lib I can still skin it to the correct color corresponding with the QPalette.

Thanks. Still shouldn’t crash though. Again we need a simple reproducer. Can you help based on that insight?

Ok, here we go:

x = QPalette.light;
y = x.deepCopy;
thisProcess.recompile;

crashes the interpreter every time on my system. And experience learns that if it crashes at recompile it could also crash earlier…

For the Menu thing I have to work a bit on that, but I can show the memory buildup of stray Menu and MenuAction objects in the QObject heap. But maybe that is already a known issue now?

Thanks, wasn’t expecting three lines!

This is the same stack trace I noted over here this morning.

I think its a QTcollider issue rather than a finalizer issue (which I think are an easy fix).

for the menus, I created this little script to check the QObject heap:

(
{
	var obj, newObj;
	{
		newObj = QObject.heap.collect(_.class).as(Set).asArray.collect({ |class|
			[ class, QObject.heap.select({ |x| x.class == class }).size ]
		}).sort({ |a,b| a[1] >= b[1] });
		if( obj != newObj ) {
			Date.localtime.postln;
			newObj.do(_.postln);
			"".postln;
			obj = newObj;
		};
		2.0.wait;
	}.loop;
}.fork( AppClock )
)

It posts whenever a change occurs in the heap, and lists the number of objects for each class. If you create a GUI window you’ll see the Views and the Window in here, and if you close it they go away again. Now, if you create a Menu like so:

x = Menu( MenuAction("hi") ).front;

you will see it generates actually two MenuActions and a Menu:

[ class MenuAction, 2 ]
[ class Menu, 1 ]

These stay there indefinitely, meaning that they are (probably) not garbage collected (also if I don’t assign the Menu to a variable as I did here). I once let this script run overnight and nothing had changed the next morning. If you do:

x.destroy

the script gives:

[ class MenuAction, 2 ]

so the Menu disappears, the two MenuActions stay. Why two, I only created one? Well, in the Menu:front method it actually creates an empty one. This is a clear and easy to fix bug. A similar thing happens when registering a Menu with MainMenu.register; it creates an extra new Menu() object for all registered MenuActions every time you register a single one (!), plus some extra MenuActions, none of them ever garbage collected.

This happens in: MainMenu:prBuildAppMenus, which is called every time the MainMenu is updated. So any kind of app menu structure in macOS will cause an enormous amount of stray menus, even if you create it only once. Note that on macOS the MainMenu is the only place to create global key commands, it is a very useful feature.

In any case, while Menu:destroy destroys the menu it doesn’t destroy the actions in it. Also, if you do destroy the MenuActions (they can of course be assigned to variables) there is still that extra MenuAction generated in Menu:front that won’t get destroyed.

In Unit-Lib I’m creating a lot of Menu’s and MenuActions on the fly, creating a very user-friendly and quick workflow with clean structures of popup menus with submenus, something that PopUpMenu can’t do. Knowing the above I’m now making sure to destroy each of them after use with my own .deepDestroy method (usually at window close), only create them at the moment when they become necessary and once created keep them in tact instead of re-creating at every click. And of course I made a version of .front that doesn’t create an extra MenuAction. For the MainMenu issue I haven’t found a fix yet, that is a bit too deep in the program to alter stuff, I once tried a ‘registerNoUpdate’ method, updating only after adding all menus, but that was not very stable. I’m not entirely sure if the build-up of Menu/MenuAction objects in the QObject heap has caused actual crashes (it could be that the crashes I had were all caused by QPalette and/or QImage) but it caused a clearly visible memory buildup over time, especially when after a while 10.000s of MenuActions were sitting there…

The uFront and deepDestroy methods I created are in the Unit-Lib quark.

cheers,
Wouter

They will only be freed when a GC flip happens, which only happens while you are using the program. You need to be allocating and freeing stuff for the garbage collector to consider releasing them.

So running this a bunch should see the numbers drop… but it doesn’t!

100000.do { Menu( MenuAction("hi") ) }

eek, surprising this has never been found before!
I think somewhere in constructObjectArray (supercollider/lang/LangPrimSource/PyrDeepCopier.h at 3b3eca75a1b42ad4a6c1146ad1893568d083d5c5 · supercollider/supercollider · GitHub) we probably should check:

} else if (isKindOf(obj, class_finalizer)) {
  // error, cannot deepCopy a finalizer
}

Or, alternatively, we just forward the original object along rather than copying - no error needed, and the behavior seems like it would work more or less as expected?
TBH I have no idea what deepCopy means on e.g. a QObject - it for sure is NOT doing what anyone thinks or would want it to do - maybe there need to be more fail cases for deepCopy?

There are already a bunch of these cases:

} else if (isKindOf(obj, class_func)) {
  putSelf(obj);
}

Maybe whats needed here is something like:

if (notCopyable(obj)) {
  putSelf(obj);
}

bool notCopyable(PyrObject* obj) {
  const auto nonCopyable = { class_func, class_method, class_thread, class_server_shm_interface, class_finalizer }; // register more at runtime?
  for (auto nonCopyClass : nonCopyable) {
    if (isKindOf(obj, nonCopyClass) return true;
  }
  return false;
}

My hunch here is that the garbage collector won’t collect stuff that is in the QObject:heap, as they are of course still available to the user from there. Could that be the issue, or is that too simple thinking ;-)?

cheers & thanks,

afaik heap is indeed a way to keep objects from being collected as long as they are in use by QT.

When a QT object is destroyed, it sends a signal here:

Which nulls the object pointer, and send an event to invalidate the sclang object:

Which calls prRelease, removing it from the heap:

On the next collect, it should call the finalizer (assuming the heap held the last reference to the SC object), which resets the pointer to the sclang object (because it’s about to be collected) and posts a DestroyProxyAndObject event:

Which is in turn processed here, deleting the QT object and the proxy:

On this code path, it seems like for delete qObject, qObject is already nullptr because of the first invalidate(), but that’s okay because this only gets called when the object is being deleted anyway.

Nothing seems problematic here, at least for QObjects? But then again: the leaking behavior is not happening for QObjectProxy, but seemingly only the OTHER QT objects that have their own finalizers.

2 Likes

The docs could be more clear though on how to deal with Menu’s that are not needed anymore, as they don’t get garbage collected. It is mentioned at .front, but the .destroy method undocumented and as said it doesn’t actually destroy the whole thing. Also, the Class code at some places seems to assume that Menu’s do get garbage collected, as it is creating quite some stray ones. To me it seems that there should at least be methods to effectively clear Menu’s and MenuActions so that they don’t bloat the memory over time…

Hi guys, sorry to bother but I’ve stumbled upon a new crash issue. It gives a different report, maybe something with VM. It’s not related to QPalette (because I’m not using that anymore for now), this is just dragging around some stuff, creating and closing some windows in Unit-Lib/WFSCollider-Class-Library. Circumstances are that I’m running SC next to Unreal engine, on an M1pro macbook with 32GB ram. SC seems very crashy suddenly, possibly related to the large memory section taken by Unreal engine (apx 10GB).

Crash report here: crash #2, not related to QPalette · GitHub

1 Like

@Wouter_Snoei maybe a good Idea would be to collect all the information and create an issue on github? Things can get lost on the forum with time.

1 Like