From b12931c84d6b23833159a41f95379a315518cdf3 Mon Sep 17 00:00:00 2001 From: Emmanuel Benoit Date: Fri, 30 Mar 2018 19:16:04 +0200 Subject: [PATCH] Filewatcher - Fixed bug caused by triggers modifying watcher state Some of the filewatcher's state was being fetched before the trigger was called and then used after that call. When the trigger modified the filewatcher's state, it would on occasion explode. --- c-filewatcher.cc | 98 ++++++++++++++++++++++++++++-------------------- 1 file changed, 57 insertions(+), 41 deletions(-) diff --git a/c-filewatcher.cc b/c-filewatcher.cc index f6ae18d..efd28cf 100644 --- a/c-filewatcher.cc +++ b/c-filewatcher.cc @@ -1,6 +1,7 @@ #include "externals.hh" #include "c-filewatcher.hh" +#define INTRUSIVE_TRACES }}} /*= T_FilesWatcher ===========================================================*/ @@ -28,78 +29,86 @@ void T_FilesWatcher::check( ) // Handle events from inotify inotify_event ie; while ( read( fd , &ie , sizeof( ie ) ) == sizeof( ie ) ) { -#ifdef INTRUSIVE_TRACES +#ifdef INTRUSIVE_TRACES // {{{ printf( "inotify wd %d / evt %x\n" , ie.wd , ie.mask ); fflush( stdout ); -#endif // INTRUSIVE_TRACES +#endif // INTRUSIVE_TRACES }}} if ( ( ie.mask & ( IN_CLOSE_WRITE | IN_DELETE_SELF ) ) == 0 ) { -#ifdef INTRUSIVE_TRACES +#ifdef INTRUSIVE_TRACES // {{{ printf( " -> skipping this event\n" ); -#endif // INTRUSIVE_TRACES +#endif // INTRUSIVE_TRACES }}} continue; } - T_FSPath const* name( inotify_.get( ie.wd ) ); - if ( !name ) { -#ifdef INTRUSIVE_TRACES + T_FSPath const* namePtr( inotify_.get( ie.wd ) ); + if ( !namePtr ) { +#ifdef INTRUSIVE_TRACES // {{{ printf( " -> no matching file\n" ); fflush( stdout ); -#endif // INTRUSIVE_TRACES +#endif // INTRUSIVE_TRACES }}} continue; } -#ifdef INTRUSIVE_TRACES - printf( " -> file '%s'\n" , name->toString( ).toOSString( ).data( ) ); -#endif // INTRUSIVE_TRACES + const auto name( *namePtr ); +#ifdef INTRUSIVE_TRACES // {{{ + printf( " -> file '%s'\n" , name.toString( ).toOSString( ).data( ) ); +#endif // INTRUSIVE_TRACES }}} - auto* entry( fileWatchers_.get( *name ) ); + auto* const entry( fileWatchers_.get( name ) ); assert( entry ); + const auto wd{ entry->wd }; entry->trigger( ); if ( ( ie.mask & IN_DELETE_SELF ) != 0 ) { -#ifdef INTRUSIVE_TRACES - printf( " -> deleted, added as missing\n" ); fflush( stdout ); -#endif // INTRUSIVE_TRACES - inotify_rm_watch( fd , entry->wd ); - missing_.add( *name ); - inotify_.remove( entry->wd ); - entry->wd = -2; +#ifdef INTRUSIVE_TRACES // {{{ + printf( " -> %s deleted, added as missing\n" , + name.toString( ).toOSString( ).data( ) ); + fflush( stdout ); +#endif // INTRUSIVE_TRACES }}} + inotify_rm_watch( fd , wd ); + missing_.add( name ); + inotify_.remove( wd ); + // We need to fetch the correct entry back, as the callback + // might have changed the underlying data. + auto* const nEntry{ fileWatchers_.get( name ) }; + assert( nEntry ); + nEntry->wd = -2; } } // Handle missing/deleted files for ( auto it = missing_.begin( ) ; it < missing_.end( ) ; ++it ) { auto const& path( *it ); -#ifdef INTRUSIVE_TRACES +#ifdef INTRUSIVE_TRACES // {{{ printf( "Checking missing file '%s'\n" , path.toString( ).toOSString( ).data( ) ); fflush( stdout ); -#endif // INTRUSIVE_TRACES +#endif // INTRUSIVE_TRACES }}} auto* const p( fileWatchers_.get( path ) ); -#ifdef INTRUSIVE_TRACES +#ifdef INTRUSIVE_TRACES // {{{ printf( " -> found? %c\n" , p ? 'Y' : 'N' ); -#endif // INTRUSIVE_TRACES +#endif // INTRUSIVE_TRACES }}} assert( p ); -#ifdef INTRUSIVE_TRACES +#ifdef INTRUSIVE_TRACES // {{{ printf( " -> wd = %d\n" , p->wd ); -#endif // INTRUSIVE_TRACES +#endif // INTRUSIVE_TRACES }}} assert( p->wd < 0 ); auto& we( *p ); const auto nwd( inotify_add_watch( fd , (char*) path.toString( ).toOSString( ).data( ) , IN_CLOSE_WRITE | IN_DELETE_SELF ) ); -#ifdef INTRUSIVE_TRACES +#ifdef INTRUSIVE_TRACES // {{{ printf( " -> inotify add watch %d\n" , nwd ); fflush( stdout ); -#endif // INTRUSIVE_TRACES +#endif // INTRUSIVE_TRACES }}} if ( nwd < 0 ) { -#ifdef INTRUSIVE_TRACES +#ifdef INTRUSIVE_TRACES // {{{ printf( " -> still missing\n" ); fflush( stdout ); -#endif // INTRUSIVE_TRACES +#endif // INTRUSIVE_TRACES }}} we.wd = -1; continue; } // Trigger if this was really a missing file if ( we.wd == -1 ) { -#ifdef INTRUSIVE_TRACES +#ifdef INTRUSIVE_TRACES // {{{ printf( " -> triggering\n" ); fflush( stdout ); -#endif // INTRUSIVE_TRACES +#endif // INTRUSIVE_TRACES }}} we.trigger( ); } @@ -127,14 +136,14 @@ void T_FilesWatcher::watchFile( T_FSPath const& path , T_WatchSet_* const watcher ) { -#ifdef INTRUSIVE_TRACES +#ifdef INTRUSIVE_TRACES // {{{ printf( "creating watch on '%s' for watcher %p\n" , path.toString( ).toOSString( ).data( ) , watcher ); fflush( stdout ); -#endif // INTRUSIVE_TRACES +#endif // INTRUSIVE_TRACES }}} auto* const exFilePos( fileWatchers_.get( path ) ); if ( exFilePos ) { -#ifdef INTRUSIVE_TRACES +#ifdef INTRUSIVE_TRACES // {{{ printf( " -> existing\n" ); fflush( stdout ); -#endif // INTRUSIVE_TRACES +#endif // INTRUSIVE_TRACES }}} auto& wl( exFilePos->watchers ); if ( !wl.contains( watcher ) ) { wl.add( watcher ); @@ -144,19 +153,26 @@ void T_FilesWatcher::watchFile( const auto fstr{ path.toString( ).toOSString( ) }; f.wd = inotify_add_watch( fd , (char*) fstr.data( ) , IN_CLOSE_WRITE | IN_DELETE_SELF ); -#ifdef INTRUSIVE_TRACES +#ifdef INTRUSIVE_TRACES // {{{ printf( " -> inotify wd %d\n" , f.wd ); fflush( stdout ); -#endif // INTRUSIVE_TRACES +#endif // INTRUSIVE_TRACES }}} if ( f.wd == -1 ) { missing_.add( path ); -#ifdef INTRUSIVE_TRACES +#ifdef INTRUSIVE_TRACES // {{{ printf( " -> missing\n" ); fflush( stdout ); -#endif // INTRUSIVE_TRACES +#endif // INTRUSIVE_TRACES }}} } else { inotify_.add( f.wd , path ); -#ifdef INTRUSIVE_TRACES +#ifdef INTRUSIVE_TRACES // {{{ printf( " -> found\n" ); fflush( stdout ); -#endif // INTRUSIVE_TRACES +#endif // INTRUSIVE_TRACES }}} + const int32_t idx{ missing_.indexOf( path ) }; + if ( idx >= 0 ) { + missing_.removeSwap( idx ); +#ifdef INTRUSIVE_TRACES // {{{ + printf( " -> removing from missing files (index %d)\n" , idx ); fflush( stdout ); +#endif // INTRUSIVE_TRACES }}} + } } f.watchers.add( watcher ); fileWatchers_.add( path , std::move( f ) );