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.
This commit is contained in:
Emmanuel BENOîT 2018-03-30 19:16:04 +02:00
parent a266120761
commit b12931c84d

View file

@ -1,6 +1,7 @@
#include "externals.hh" #include "externals.hh"
#include "c-filewatcher.hh" #include "c-filewatcher.hh"
#define INTRUSIVE_TRACES }}}
/*= T_FilesWatcher ===========================================================*/ /*= T_FilesWatcher ===========================================================*/
@ -28,78 +29,86 @@ void T_FilesWatcher::check( )
// Handle events from inotify // Handle events from inotify
inotify_event ie; inotify_event ie;
while ( read( fd , &ie , sizeof( ie ) ) == sizeof( 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 ); 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 ) { if ( ( ie.mask & ( IN_CLOSE_WRITE | IN_DELETE_SELF ) ) == 0 ) {
#ifdef INTRUSIVE_TRACES #ifdef INTRUSIVE_TRACES // {{{
printf( " -> skipping this event\n" ); printf( " -> skipping this event\n" );
#endif // INTRUSIVE_TRACES #endif // INTRUSIVE_TRACES }}}
continue; continue;
} }
T_FSPath const* name( inotify_.get( ie.wd ) ); T_FSPath const* namePtr( inotify_.get( ie.wd ) );
if ( !name ) { if ( !namePtr ) {
#ifdef INTRUSIVE_TRACES #ifdef INTRUSIVE_TRACES // {{{
printf( " -> no matching file\n" ); fflush( stdout ); printf( " -> no matching file\n" ); fflush( stdout );
#endif // INTRUSIVE_TRACES #endif // INTRUSIVE_TRACES }}}
continue; continue;
} }
#ifdef INTRUSIVE_TRACES const auto name( *namePtr );
printf( " -> file '%s'\n" , name->toString( ).toOSString( ).data( ) ); #ifdef INTRUSIVE_TRACES // {{{
#endif // 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 ); assert( entry );
const auto wd{ entry->wd };
entry->trigger( ); entry->trigger( );
if ( ( ie.mask & IN_DELETE_SELF ) != 0 ) { if ( ( ie.mask & IN_DELETE_SELF ) != 0 ) {
#ifdef INTRUSIVE_TRACES #ifdef INTRUSIVE_TRACES // {{{
printf( " -> deleted, added as missing\n" ); fflush( stdout ); printf( " -> %s deleted, added as missing\n" ,
#endif // INTRUSIVE_TRACES name.toString( ).toOSString( ).data( ) );
inotify_rm_watch( fd , entry->wd ); fflush( stdout );
missing_.add( *name ); #endif // INTRUSIVE_TRACES }}}
inotify_.remove( entry->wd ); inotify_rm_watch( fd , wd );
entry->wd = -2; 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 // Handle missing/deleted files
for ( auto it = missing_.begin( ) ; it < missing_.end( ) ; ++it ) { for ( auto it = missing_.begin( ) ; it < missing_.end( ) ; ++it ) {
auto const& path( *it ); auto const& path( *it );
#ifdef INTRUSIVE_TRACES #ifdef INTRUSIVE_TRACES // {{{
printf( "Checking missing file '%s'\n" , path.toString( ).toOSString( ).data( ) ); fflush( stdout ); printf( "Checking missing file '%s'\n" , path.toString( ).toOSString( ).data( ) ); fflush( stdout );
#endif // INTRUSIVE_TRACES #endif // INTRUSIVE_TRACES }}}
auto* const p( fileWatchers_.get( path ) ); auto* const p( fileWatchers_.get( path ) );
#ifdef INTRUSIVE_TRACES #ifdef INTRUSIVE_TRACES // {{{
printf( " -> found? %c\n" , p ? 'Y' : 'N' ); printf( " -> found? %c\n" , p ? 'Y' : 'N' );
#endif // INTRUSIVE_TRACES #endif // INTRUSIVE_TRACES }}}
assert( p ); assert( p );
#ifdef INTRUSIVE_TRACES #ifdef INTRUSIVE_TRACES // {{{
printf( " -> wd = %d\n" , p->wd ); printf( " -> wd = %d\n" , p->wd );
#endif // INTRUSIVE_TRACES #endif // INTRUSIVE_TRACES }}}
assert( p->wd < 0 ); assert( p->wd < 0 );
auto& we( *p ); auto& we( *p );
const auto nwd( inotify_add_watch( fd , const auto nwd( inotify_add_watch( fd ,
(char*) path.toString( ).toOSString( ).data( ) , (char*) path.toString( ).toOSString( ).data( ) ,
IN_CLOSE_WRITE | IN_DELETE_SELF ) ); IN_CLOSE_WRITE | IN_DELETE_SELF ) );
#ifdef INTRUSIVE_TRACES #ifdef INTRUSIVE_TRACES // {{{
printf( " -> inotify add watch %d\n" , nwd ); fflush( stdout ); printf( " -> inotify add watch %d\n" , nwd ); fflush( stdout );
#endif // INTRUSIVE_TRACES #endif // INTRUSIVE_TRACES }}}
if ( nwd < 0 ) { if ( nwd < 0 ) {
#ifdef INTRUSIVE_TRACES #ifdef INTRUSIVE_TRACES // {{{
printf( " -> still missing\n" ); fflush( stdout ); printf( " -> still missing\n" ); fflush( stdout );
#endif // INTRUSIVE_TRACES #endif // INTRUSIVE_TRACES }}}
we.wd = -1; we.wd = -1;
continue; continue;
} }
// Trigger if this was really a missing file // Trigger if this was really a missing file
if ( we.wd == -1 ) { if ( we.wd == -1 ) {
#ifdef INTRUSIVE_TRACES #ifdef INTRUSIVE_TRACES // {{{
printf( " -> triggering\n" ); fflush( stdout ); printf( " -> triggering\n" ); fflush( stdout );
#endif // INTRUSIVE_TRACES #endif // INTRUSIVE_TRACES }}}
we.trigger( ); we.trigger( );
} }
@ -127,14 +136,14 @@ void T_FilesWatcher::watchFile(
T_FSPath const& path , T_FSPath const& path ,
T_WatchSet_* const watcher ) 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 ); 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 ) ); auto* const exFilePos( fileWatchers_.get( path ) );
if ( exFilePos ) { if ( exFilePos ) {
#ifdef INTRUSIVE_TRACES #ifdef INTRUSIVE_TRACES // {{{
printf( " -> existing\n" ); fflush( stdout ); printf( " -> existing\n" ); fflush( stdout );
#endif // INTRUSIVE_TRACES #endif // INTRUSIVE_TRACES }}}
auto& wl( exFilePos->watchers ); auto& wl( exFilePos->watchers );
if ( !wl.contains( watcher ) ) { if ( !wl.contains( watcher ) ) {
wl.add( watcher ); wl.add( watcher );
@ -144,19 +153,26 @@ void T_FilesWatcher::watchFile(
const auto fstr{ path.toString( ).toOSString( ) }; const auto fstr{ path.toString( ).toOSString( ) };
f.wd = inotify_add_watch( fd , (char*) fstr.data( ) , f.wd = inotify_add_watch( fd , (char*) fstr.data( ) ,
IN_CLOSE_WRITE | IN_DELETE_SELF ); IN_CLOSE_WRITE | IN_DELETE_SELF );
#ifdef INTRUSIVE_TRACES #ifdef INTRUSIVE_TRACES // {{{
printf( " -> inotify wd %d\n" , f.wd ); fflush( stdout ); printf( " -> inotify wd %d\n" , f.wd ); fflush( stdout );
#endif // INTRUSIVE_TRACES #endif // INTRUSIVE_TRACES }}}
if ( f.wd == -1 ) { if ( f.wd == -1 ) {
missing_.add( path ); missing_.add( path );
#ifdef INTRUSIVE_TRACES #ifdef INTRUSIVE_TRACES // {{{
printf( " -> missing\n" ); fflush( stdout ); printf( " -> missing\n" ); fflush( stdout );
#endif // INTRUSIVE_TRACES #endif // INTRUSIVE_TRACES }}}
} else { } else {
inotify_.add( f.wd , path ); inotify_.add( f.wd , path );
#ifdef INTRUSIVE_TRACES #ifdef INTRUSIVE_TRACES // {{{
printf( " -> found\n" ); fflush( stdout ); 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 ); f.watchers.add( watcher );
fileWatchers_.add( path , std::move( f ) ); fileWatchers_.add( path , std::move( f ) );