Mixer: Fix crash when closing project containing certain configurations of modules.

mixer/src/Chain.C

@@ -174,7 +174,9 @@ Chain::~Chain ( )
 
     _deleting = true;
 
-    client()->lock();
+    /* FIXME: client may already be dead during teardown if group is destroyed first. */
+    if ( client() )
+	client()->lock();
 
     for ( unsigned int i = scratch_port.size(); i--; )
         free( (sample_t*)scratch_port[i].buffer() );
@@ -182,9 +184,12 @@ Chain::~Chain ( )
     /* if we leave this up to FLTK, it will happen after we've
      already destroyed the client */
     modules_pack->clear();
+    modules_pack = NULL;
     controls_pack->clear();
+    modules_pack = NULL;
 
-    client()->unlock();
+    if ( client() )
+	client()->unlock();
 }
 
 Group *
@@ -534,6 +539,8 @@ Chain::name ( const char *name )
     if ( strip()->group() )
     {
         if ( strip()->group()->single() )
+	    /* we are the owner of this group and its only member, so
+	     * rename it */
             strip()->group()->name(name);
     }
     
@@ -713,6 +720,8 @@ Chain::add_to_process_queue ( Module *m )
 void
 Chain::build_process_queue ( void )
 {
+    client()->lock();
+    
     process_queue.clear();
 
     for ( int i = 0; i < modules(); ++i )
@@ -780,6 +789,8 @@ Chain::build_process_queue ( void )
 /*             delete[] s; */
 /*         } */
 /*     } */
+
+    client()->unlock();
 }
 
 void
@@ -901,9 +912,9 @@ Chain::process ( nframes_t nframes )
 {
     for ( std::list<Module*>::const_iterator i = process_queue.begin(); i != process_queue.end(); ++i )
     {
-        Module *m = *i;
+	Module *m = *i;
-
+	    
-        m->process( nframes );
+	m->process( nframes );
     }
 }
 

mixer/src/Chain.H

@@ -109,7 +109,7 @@ public:
 
     bool can_support_input_channels ( int n );
 
-    int modules ( void ) const { return modules_pack->children(); }
+    int modules ( void ) const { return modules_pack ? modules_pack->children() : 0; }
     Module *module ( int n ) const { return (Module*)modules_pack->child( n ); }
     void remove ( Controller_Module *m );
     void remove ( Module *m );

mixer/src/Controller_Module.C

@@ -96,8 +96,6 @@ Controller_Module::~Controller_Module ( )
 
     /* shutdown JACK port, if we have one */
     mode( GUI );
-
-//    disconnect();
 }
 
 void

mixer/src/Group.C

@@ -56,9 +56,18 @@ Group::~Group ( )
 {
     DMESSAGE( "Destroying group" );
 
+    for ( std::list<Mixer_Strip*>::iterator i = strips.begin();
+          i != strips.end();
+          i++ )
+    {
+	/* avoid a use after free during project close when the group
+	 * may be destroyed before its member strips are */
+	(*i)->clear_group();
+    }
+
     if ( _name )
         free( _name );
-
+    
     deactivate();
 }
 
@@ -101,12 +110,16 @@ Group::set ( Log_Entry &e )
 void
 Group::latency ( jack_latency_callback_mode_t mode )
 {
-    for ( std::list<Mixer_Strip*>::iterator i = strips.begin();
+    if ( trylock() )
-          i != strips.end();
-          i++ )
     {
-        if ( (*i)->chain() )            
+	for ( std::list<Mixer_Strip*>::iterator i = strips.begin();
-            (*i)->chain()->set_latency(mode == JackCaptureLatency ? JACK::Port::Input : JACK::Port::Output );
+	      i != strips.end();
+	      i++ )
+	{
+	    if ( (*i)->chain() )            
+		(*i)->chain()->set_latency(mode == JackCaptureLatency ? JACK::Port::Input : JACK::Port::Output );
+	}
+	unlock();
     }
 }
 
@@ -208,8 +221,9 @@ Group::process ( nframes_t nframes )
 void
 Group::recal_load_coef ( void )
 {
-    _load_coef = 1.0f / ( nframes() / (float)sample_rate() * 1000000.0 );
+    _load_coef = 1.0f / ( nframes() / (float)sample_rate() * 1000000.0f );
 }
+
 int
 Group::sample_rate_changed ( nframes_t srate )
 {
@@ -273,6 +287,7 @@ void
 Group::add ( Mixer_Strip *o )
 {
     lock();
+
     if ( ! active() )
     {
         /* to call init */
@@ -280,10 +295,12 @@ Group::add ( Mixer_Strip *o )
         name(n);
         free(n);
     }
+
     if ( o->chain() )
         o->chain()->thaw_ports();
 
     strips.push_back(o);
+
     unlock();
 }
 
@@ -291,13 +308,15 @@ void
 Group::remove ( Mixer_Strip *o )
 {
     lock();
+    
     strips.remove(o);
+    
     if ( o->chain() )
         o->chain()->freeze_ports();
+
     if ( strips.size() == 0 && active() )
-    {
         Client::close();
-    }
+    
     unlock();
 }
 

mixer/src/Group.H

@@ -97,6 +97,8 @@ public:
     void add (Mixer_Strip*);
     void remove (Mixer_Strip*);
 
+    int children ( void ) const { return strips.size(); }
+    
     /* Engine *engine ( void ) { return _engine; } */
 };
 

mixer/src/Mixer.C

@@ -821,8 +821,6 @@ void Mixer::remove ( Mixer_Strip *ms )
 
     mixer_strips->remove( ms );
     
-    ms->group()->remove( ms );
-
     if ( parent() )
         parent()->redraw();
 }

mixer/src/Mixer_Strip.C

@@ -73,6 +73,7 @@ Mixer_Strip::Mixer_Strip( const char *strip_name ) : Fl_Group( 0, 0, 120, 600 )
 
     init();
 
+    /* create single member group */
     _group = new Group(strip_name, true);
 
     _group->add( this );
@@ -105,14 +106,30 @@ Mixer_Strip::~Mixer_Strip ( )
 //    _chain->engine()->lock();
 
     log_destroy();
-
+    
     mixer->remove( this );
 
     /* make sure this gets destroyed before the chain */
     fader_tab->clear();
 
-    delete _chain;
+    if ( _group )
-    _chain = NULL;
+    {
+	_group->remove( this );
+    }
+    
+    if ( _chain )
+    {
+	delete _chain;
+	_chain = NULL;
+    }
+
+    if ( _group )
+    {
+	if ( 0 == _group->children() )
+	    delete _group;
+	
+	_group = NULL;
+    }
 }
 
 
@@ -347,6 +364,7 @@ void Mixer_Strip::cb_handle(Fl_Widget* o, void* v) {
 void
 Mixer_Strip::group ( Group *g )
 {
+    /* FIXME: what is the intention here? */
     if ( !g && _group && _group->single() )
         return;
 

mixer/src/Mixer_Strip.H

@@ -169,7 +169,8 @@ public:
 
     Controller_Module *spatializer ( void );
 
-    Group *group ( void ) { return _group;}
+    Group *group ( void ) { return _group; }
+    void clear_group ( void ) { _group = NULL; }
 
     //  int group ( void ) const;
     void group ( Group * );

mixer/src/Module.C

@@ -104,27 +104,36 @@ Module::~Module ( )
         if ( control_input[i].connected() )
         {
             Module *o = (Module*)control_input[i].connected_port()->module();
-            
-            if ( ! o->is_default() )
-            {
-                control_input[i].disconnect();
-
-                DMESSAGE( "Deleting connected module %s", o->label() );
-                
-                delete o;
-            }
-            else
-            {
-                control_input[i].disconnect();
-            }
-
-        }
 
+	    if ( !o )
+	    {
+		DWARNING( "Programming error. Connected port has null module. %s %s",
+			 
+			  label(),
+			  control_input[i].connected_port()->name());
+	    }
+	    
+	    control_input[i].disconnect();
+	    
+	    if ( o )
+	    {
+		if ( ! o->is_default() )
+		{
+		    DMESSAGE( "Deleting connected module %s", o->label() );
+		    
+		    delete o;
+		}
+	    }
+	}
+	
         control_input[i].destroy_osc_port();
     }
     for ( unsigned int i = 0; i < control_output.size(); ++i )
         control_output[i].disconnect();
 
+    aux_audio_output.clear();
+    aux_audio_input.clear();
+    
     audio_input.clear();
     audio_output.clear();
 
@@ -1180,7 +1189,19 @@ Module::freeze_ports ( void )
     for ( int i = 0; i < ncontrol_inputs(); ++i )
     {
         if ( control_input[i].connected() )
-            control_input[i].connected_port()->module()->freeze_ports();
+	{
+	    if ( ! control_input[i].connected_port()->module()  )
+	    {
+		DWARNING( "Programming error. Connected port has null module. %s %s",
+			 
+			 name(),
+			 control_input[i].connected_port()->name());
+	    }
+	    else
+	    {
+		control_input[i].connected_port()->module()->freeze_ports();
+	    }
+	}
     }
 
     for ( unsigned int i = 0; i < aux_audio_input.size(); ++i )

mixer/src/Module.H

@@ -173,6 +173,9 @@ public:
 
         virtual ~Port ( )
             {
+		/* FIXME: will this cause problems with cloning an instance? */
+		disconnect();
+		
                 if ( _by_number_path )
                     free( _by_number_path );
                 _by_number_path = NULL;
@@ -321,20 +324,10 @@ public:
         /* disconnect from *all* connected ports */
         void disconnect ( void )
             {
-                if ( _connected.size() )
+		while ( _connected.size() )
-                {
+		    disconnect(_connected.front());
-                    for ( std::list<Port*>::iterator i = _connected.begin(); i != _connected.end(); i++ )
+	    }
-                    {
+	
-                        Port *p = *i;
-
-                        /* iterator about to be invalidated... */
-                        i = _connected.erase(i);
-                        
-                        disconnect(p);
-                    }
-                }
-            }
-
         void jack_port ( JACK::Port *v ) { _jack_port = v; }
         JACK::Port *jack_port ( void ) const { return _jack_port; }