Timeline: Lock ordering fixes.

timeline/src/Engine/Disk_Stream.C

@@ -146,10 +146,18 @@ Disk_Stream::shutdown ( void )
         _terminate = true;
         
         /* try to wake the thread so it'll see that it's time to die */
+        int total_ms = 0;
         while ( _terminate )
         {
-            usleep( 100 );
+            usleep( 1000 * 10 );
+            total_ms += 10;
             block_processed();
+            
+            if ( total_ms > 100 )
+            {
+                WARNING("Disk_Stream thread has taken longer than %ims to respond to terminate signal.", total_ms );
+                break;
+            }
         }
         
         _thread.join();

timeline/src/Engine/Playback_DS.C

@@ -82,25 +82,22 @@ Playback_DS::read_block ( sample_t *buf, nframes_t nframes )
 
 //    printf( "IO: attempting to read block @ %lu\n", _frame );
 
-    if ( ! sequence() )
-    {
-        /* FIXME: what to do here? */
-//        _frame += _nframes;
-        return;
-    }
-
     timeline->rdlock();
 
-    /* FIXME: how does this work if _delay is not a multiple of bufsize? */
-
-    if ( _frame >= _delay )
+    if ( sequence() )
     {
-        if ( ! sequence()->play( buf, _frame - _delay, nframes, channels() ) )
-            WARNING( "Programming error?" );
+        /* FIXME: how does this work if _delay is not a multiple of bufsize? */
+        
+        if ( _frame >= _delay )
+        {
+            if ( ! sequence()->play( buf, _frame - _delay, nframes, channels() ) )
+                WARNING( "Programming error?" );
+        }
+        
+        _frame += nframes;
+        
     }
-
-    _frame += nframes;
-
+    
     timeline->unlock();
 }
 

timeline/src/Engine/Record_DS.C

@@ -58,13 +58,9 @@ Record_DS::write_block ( sample_t *buf, nframes_t nframes )
     if ( ! ( timeline && sequence() ) )
         return;
 
-//    timeline->wrlock();
-
     track()->write( _capture, buf, nframes );
 
     _frames_written += nframes;
-
-//    timeline->unlock();
 }
 
 #define AVOID_UNNECESSARY_COPYING 1

timeline/src/Engine/Timeline.C

@@ -150,6 +150,10 @@ Timeline::stop ( void )
 nframes_t
 Timeline::process ( nframes_t nframes )
 {
+    /* there is no need to acquire a readlock here because track *
+       addition/removal locks process() and track process() calls deal with
+       ringbuffers instead of reading the sequence data directly.  */
+
     for ( int i = tracks->children(); i-- ; )
     {
         Track *t = (Track*)tracks->child( i );

timeline/src/Engine/Track.C

@@ -272,8 +272,14 @@ Track::record ( Capture *c, nframes_t frame )
     /* open it again for reading in the GUI thread */
     //   Audio_File *af = Audio_File::from_file( c->audio_file->name() );
 
+    /* must acquire a write lock because the Audio_Region constructor
+     * will add the region to the specified sequence */
+    timeline->wrlock();
+
     c->region = new Audio_Region( c->audio_file, sequence(), frame );
 
+    timeline->unlock();
+
     c->region->prepare();
 }
 
@@ -298,6 +304,8 @@ Track::finalize ( Capture *c, nframes_t frame )
     /* adjust region start for latency */
     /* FIXME: is just looking at the first channel good enough? */
 
+    timeline->wrlock();
+
     DMESSAGE( "finalizing audio file" );
     /* must finalize audio before peaks file, otherwise another thread
      * might think the peaks are out of date and attempt to regenerate
@@ -323,5 +331,7 @@ Track::finalize ( Capture *c, nframes_t frame )
 
     c->region->offset( capture_offset );
 
+    timeline->unlock();
+
 //    delete c->audio_file;
 }

timeline/src/Sequence.C

@@ -154,8 +154,12 @@ Sequence::overlaps ( Sequence_Widget *r )
 void
 Sequence::handle_widget_change ( nframes_t start, nframes_t length )
 {
+    /* this might be invoked from Capture or GUI thread */
+    Fl::lock();
     sort();
     timeline->damage_sequence();
+    Fl::unlock();
+
 //    timeline->update_length( start + length );
 }
 
@@ -186,8 +190,11 @@ Sequence::add ( Sequence_Widget *r )
 
     if ( r->sequence() )
     {
+        /* This method can be called from the Capture thread as well as the GUI thread, so we must lock FLTK before redraw */
+        Fl::lock();
         r->redraw();
         r->sequence()->remove( r );
+        Fl::unlock();
 //        r->track()->redraw();
     }
 
@@ -204,6 +211,7 @@ Sequence::remove ( Sequence_Widget *r )
     _widgets.remove( r );
 
     handle_widget_change( r->start(), r->length() );
+    
 }
 
 static nframes_t

timeline/src/TLE.fl

@@ -943,6 +943,10 @@ else if ( 0 == p )
 	progress_group->hide();
 }
 
+// never allow drawing timeline while loading, as this would cause lock recursion.
+if ( timeline->visible() )
+   return;
+
 static int oldp;
 static char pat[10];
 

timeline/src/Timeline.C

@@ -1253,6 +1253,8 @@ Timeline::draw ( void )
     int bdx = 0;
     int bdw = 0;
 
+    rdlock();
+
     X = tracks->x() + bdx + 1;
     Y = tracks->y();
     W = tracks->w() - bdw - 1;
@@ -1354,6 +1356,8 @@ done:
 
     _old_xposition = xoffset;
     _old_yposition = panzoomer->y_value();
+
+    unlock();
 }
 
 void

timeline/src/Transport.H

@@ -63,8 +63,8 @@ public:
     void toggle_record ( void );
     int handle ( int m );
 
-    bool rolling;
-    bool recording;
+    volatile bool rolling;
+    volatile bool recording;
 
     void poll ( void );
     void locate ( nframes_t frame );

timeline/src/main.C

@@ -280,8 +280,9 @@ main ( int argc, char **argv )
         if ( optind < argc )
         {
             MESSAGE( "Loading \"%s\"", argv[optind] );
-            
+            Fl::lock();
             tle->open( argv[optind] );
+            Fl::unlock();
         }
     }