On 09.09.2024 21:32, ichthyo wrote:
*Kristian*: could you help me reduce the attack surface?
On 11.09.24 21:11, Kristian Amlie wrote:
Alright, I finally got around to this: - kristian_1 - Works! - kristian_2 - Works! - guiconnect - Crashes (*)
Wow -- this completely changes the situation. Thanks Kristian! It means, that both the actual core of the instance handling rework and the LV2 rework do not cause the problem, and also the Fix for the EQ-Graph is fine. Rather, the problem must be hidden somewhere in the additional clean-up work. Part of this clean-up is a direct follow-up of the refactoring, like removing code and functionality no longer required after the switch to the InstanceManager. But beyond that, since some time I also try to drive the modernisation of memory-management ahead, which is tedious and requires lots of fidgeting. Basically the idea here is, to replace the fragile, hand-written destructor and clean-up logic by features of modern C++, which work automatically and reliably in all cases. Notably, the original code was written not in C++, but in some C-with-classes dialect. Judging from the clean-up logic scattered all over the place, the allocator used at that time behaved similar to the C "malloc", which means, it can return a NULL pointer on out-of-memory. However, C++ behaves completely different: it *never* returns NULL from an operator new, rather it throws an std::bad_alloc exception. Thus, after Yoshimi was switched to C++ - the whole code base is littered with quite complex clean-up and failure handling logic, which can *never* actually be activated - whenever an out-of-memory happens, Yoshimi will happily leak memory all over the place, and typically go into a deadlocked state, since also back-ends are not closed and background threads continue to work. The obvious solution, which I am driving ahead in small increments, is to replace manual memory management by smart-pointers (unique_ptr, shared_ptr), which are exception safe and can never leak memory. The third topic, which I try to address in small increments, is to get rid of that jungle of SynthEngine pointers, which are used all over the place and make it damn hard to assess any actual data dependencies. Basically, the SynthEngine and the connected "Runtime" is a "God Class", where a shitload of unrelated things are stuffed in, because it is so convenient to have them available from "everywhere" The first step to resolve such excessively tangled code, is to separate into several concerns, which can be addressed more directly. - in some cases, pointers were also used to implement an "optional" behaviour - in some further cases, actually something can be a singleton or global var. - in many cases, code does not need the SynthEngine, but rather the Config or the InterChange. OK, *so how to proceed now*? Since we are headed towards a release, I will weed out that clean-up changeset, and try to single out the immediate follow-up changes. All further changes could be held back and revisited after the release, even more so, since now also the Effect section in the SynthEngine could be simplified a lot, as the GUI does not need to access Effects directly any more. -- Herman
(*) Note that the latest one needed some std namespace fixes, just FYI.
Yes, noticed that already. It was a std::endl, which was covered by an import in one of my additional debugging helpers on my work branch. _______________________________________________ Yoshimi-devel mailing list Yoshimi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
