zynaddsubfx

commit 47680e27ebcab6c724f3ec195ca89b3098866281
parent 0a5445867e602672beb45a2d1517abed414fbe25
Author: Johannes Lorenz <j.git@lorenz-ho.me>
Date:   Sun, 28 Jul 2019 23:40:01 +0200

Fix crash when loading while backend is dead

MiddleWare can request Master to reload (from a blob), and it can also take
over OSC message handling of the Master if the backend is dead. If the
former happens while the backend is dead, and then the latter happens,
MiddleWare by now calls Master::runOsc() on the new master, telling it to
exchange with the new master, and then deleting the new master.
Consequently, the `MiddleWareImpl::master` points to deleted memory, leading
to segfaults in DAWs where the audio thread is not active while loading the
plugin preset.

If now, with this commit, MiddleWare runs Master, it passes its previous
Master to runOsc, so runOsc then considers the previous master as "this", so
it does a real exchange and requests to free the previous master.

Diffstat:
M
src/Misc/Master.cpp
 | 
22
++++++++++++++++------
M
src/Misc/Master.h
 | 
9
++++++---
M
src/Misc/MiddleWare.cpp
 | 
14
+++++++++++---

3 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/src/Misc/Master.cpp b/src/Misc/Master.cpp
@@ -776,17 +776,26 @@ Master::Master(const SYNTH_T &synth_, Config* config)
 }
 
 bool Master::applyOscEvent(const char *msg, float *outl, float *outr,
-                           bool offline, bool nio, DataObj& d, int msg_id)
+                           bool offline, bool nio, DataObj& d, int msg_id,
+                           Master* master_from_mw)
 {
     if(!strcmp(msg, "/load-master")) {
-        Master *this_master = this;
+        Master *this_master = master_from_mw ? master_from_mw : this;
         Master *new_master  = *(Master**)rtosc_argument(msg, 0).b.data;
+        // This can not fail anymore, but just to be sure...
+        assert(new_master != this_master);
+
+        /*
+         * WARNING: Do not use anything from "this" below, use "this_master"
+         */
+
         if(!offline)
             new_master->AudioOut(outl, outr);
         if(nio)
             Nio::masterSwap(new_master);
-        if (hasMasterCb())
-            mastercb(mastercb_ptr, new_master);
+        if (this_master->hasMasterCb()) {
+            this_master->mastercb(this_master->mastercb_ptr, new_master);
+        }
         bToU->write("/free", "sb", "Master", sizeof(Master*), &this_master);
         return false;
     } else if(!strcmp(msg, "/switch-master")) {
@@ -1121,7 +1130,8 @@ void dump_msg(const char* ptr, std::ostream& os = std::cerr)
 #endif
 int msg_id=0;
 
-bool Master::runOSC(float *outl, float *outr, bool offline)
+bool Master::runOSC(float *outl, float *outr, bool offline,
+                    Master* master_from_mw)
 {
     // the following block is only ever entered by 1 thread at a time
     // other threads have to ignore it
@@ -1141,7 +1151,7 @@ bool Master::runOSC(float *outl, float *outr, bool offline)
         {
             const char *msg = uToB->read();
             if(! applyOscEvent(msg, outl, outr, offline, true, d, msg_id,
-                               ) )
+                               master_from_mw) )
             {
                 run_osc_in_use.store(false);
                 return false;
diff --git a/src/Misc/Master.h b/src/Misc/Master.h
@@ -121,7 +121,8 @@ class Master
         void vuUpdate(const float *outl, const float *outr);
 
         //Process a set of OSC events in the bToU buffer
-        bool runOSC(float *outl, float *outr, bool offline=false);
+        bool runOSC(float *outl, float *outr, bool offline=false,
+                    Master* master_from_mw = nullptr);
 
         /**Audio Output*/
         bool AudioOut(float *outl, float *outr) REALTIME;
@@ -237,10 +238,12 @@ class Master
         //Used by loadOSC and saveOSC
         int loadOSCFromStr(const char *file_content,
                            rtosc::savefile_dispatcher_t* dispatcher);
-        //applyOscEvent with a DataObj parameter
+        //!applyOscEvent with a DataObj parameter
+        //!@return false iff master has been changed
         bool applyOscEvent(const char *event, float *outl, float *outr,
                            bool offline, bool nio,
-                           class DataObj& d, int msg_id = -1);
+                           class DataObj& d, int msg_id = -1,
+                           Master* master_from_mw = nullptr);
 };
 
 class master_dispatcher_t : public rtosc::savefile_dispatcher_t
diff --git a/src/Misc/MiddleWare.cpp b/src/Misc/MiddleWare.cpp
@@ -578,6 +578,7 @@ public:
         //Update resource locator table
         updateResources(m);
 
+        previous_master = master;
         master = m;
 
         //Give it to the backend and wait for the old part to return for
@@ -696,10 +697,13 @@ public:
 
         heartBeat(master);
 
-        //XXX This might have problems with a master swap operation
         if(offline)
-            master->runOSC(0,0,true);
-
+        {
+            //pass previous master in case it will have to be freed
+            //similar to previous_master->runOSC(0,0,true)
+            //but note that previous_master could have been freed already
+            master->runOSC(0,0,true, previous_master);
+        }
     }
 
 
@@ -741,6 +745,10 @@ public:
     //this assumption is broken
     Master *master;
 
+    //The master before the last load operation, if any
+    //Only valid until freed
+    Master *previous_master = nullptr;
+
     //The ONLY means that any chunk of UI code should have for interacting with the
     //backend
     Fl_Osc_Interface *osc;