On Mon, 2 Dec 2024 20:43:35 +0100 ichthyo <[email protected]> wrote:
>On 02.12.24 16:00, ichthyo wrote: >> So I am not really happy with that solution, since all hinges on such >> a long and convoluted chain of arguments, with lots of subtle points. >> And even more so, since we'd have to build a similar argument for the >> stand-alone case too. > >Hi Yoshimi-devs, > >all of this discussion was quite helpful for me to understand that >we have two interwoven issues: > >- initialise the communication when starting the GUI >- ensure that both threads see the data used for bootstrapping. > >Looking at how the GUI is actually launched, it becomes clear that >all these state transitions happen from within the InstanceManager. >Even more so, they already happen from the private part. > >So in order to solve the *thread visibiltiy* issue, it is sufficient >to slightly adjust the private methods within InstanceManager so to >ensure that all call paths are protected by the InstanceManager mutex. >I had added this Mutex last summer to protect the setup and tear-down >of Instances, since none of this is performance critical. > >This seems a much cleaner solution for me, instead of any convoluted >argument why some access path just happens to be thread safe, by >relying on some fragile and non-obvious circumstances. > > >The other part turned out to be much more "interesting". >Because the push-update was rather integrated deeply into the >baseclass of MasterUI. Since it is now clear that the access to >create the UI happens only over one access point and is within >a safe zone, we can completely sidestep all of the ring buffer >and messaging stuff. It is also clear that we need this only >one time when creating the MasterUI object, so we actually >do not need push-Updates. > >Thus, by re-thinking this whole aspect, we figured out that >this is a special case and that it was a rather bad idea to >attempt to subsume it under the general case of UI push-updates. >Just look at the change sets -- it is much simpler now. > >My apologies for that; >seemingly I was totally preoccupied by the general case. > > >I made this change in two commits. >They are in my Github repo, on Master and I will send a pull request. > >-- Hermann I've just completed a very thorough test in three different machines (none of them less than 5 years old) a quad core (dual 'real' cores) Intel laptop, a dual core small desktop and a AMD Ryzen 5. None of them had any problems. I was quite brutal with these, faking complete new installs, deleting various parts of yoshimi's config directory and corrupting the bank structure. They all passed without apparent difficulties. I've had a problem with a Raspberry Pi, in that it's not recognising an all ALSA setup, but on further testing, that's true of the previous Yoshimi version. I'll have to investigate that separately. In all other respects it's fine. I've not had a really good look at the code changes yet, but on a quick glance it looks cleaner. Unless something else crops up, I'll make a bugfix release (2.3.3.2) this evening. -- Will J Godfrey _______________________________________________ Yoshimi-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
