From cd55994f91415236a7346b771d5222f9458d0af2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emmanuel=20Beno=C3=AEt?= Date: Sun, 5 Nov 2017 09:55:37 +0100 Subject: [PATCH] Fixed bug in filewatcher --- filewatcher.cc | 133 ++++++++++++++++++++++++++++--------------------- filewatcher.hh | 30 ++++++----- 2 files changed, 94 insertions(+), 69 deletions(-) diff --git a/filewatcher.cc b/filewatcher.cc index 03f60d6..f012206 100644 --- a/filewatcher.cc +++ b/filewatcher.cc @@ -9,59 +9,55 @@ T_FilesWatcher::T_FilesWatcher( ) : fd( inotify_init1( O_NONBLOCK ) ) { } -T_FilesWatcher::T_FilesWatcher( T_FilesWatcher&& other ) noexcept - : fd( 0 ) , watched_( std::move( other.watched_ ) ) -{ - std::swap( fd , other.fd ); - other.watched_.clear( ); - for ( T_WatchedFiles* wf : watched_ ) { - if ( wf ) { - wf->watcher = this; - } - } -} - T_FilesWatcher::~T_FilesWatcher( ) { if ( fd ) { close( fd ); } - for ( T_WatchedFiles* wf : watched_ ) { - if ( wf ) { - wf->watcher = nullptr; - } + for ( T_WatchSet_* wf : watched_ ) { + wf->watcher = nullptr; } } void T_FilesWatcher::check( ) { // Reset triggered status - for ( T_WatchedFiles* wf : watched_ ) { + for ( T_WatchSet_* wf : watched_ ) { wf->triggered = false; } // Handle events from inotify inotify_event ie; while ( read( fd , &ie , sizeof( ie ) ) == sizeof( ie ) ) { -// printf( "inotify wd %d / evt %x\n" , ie.wd , ie.mask ); fflush( stdout ); +#ifdef INTRUSIVE_TRACES + printf( "inotify wd %d / evt %x\n" , ie.wd , ie.mask ); fflush( stdout ); +#endif // INTRUSIVE_TRACES if ( ( ie.mask & ( IN_CLOSE_WRITE | IN_DELETE_SELF ) ) == 0 ) { -// printf( " -> skipping this event\n" ); +#ifdef INTRUSIVE_TRACES + printf( " -> skipping this event\n" ); +#endif // INTRUSIVE_TRACES continue; } T_String const* name( inotify_.get( ie.wd ) ); if ( !name ) { -// printf( " -> no matching file\n" ); fflush( stdout ); +#ifdef INTRUSIVE_TRACES + printf( " -> no matching file\n" ); fflush( stdout ); +#endif // INTRUSIVE_TRACES continue; } +#ifdef INTRUSIVE_TRACES printf( " -> file '%s'\n" , name->toOSString( ).data( ) ); +#endif // INTRUSIVE_TRACES auto* entry( fileWatchers_.get( *name ) ); assert( entry ); entry->trigger( ); if ( ( ie.mask & IN_DELETE_SELF ) != 0 ) { - //printf( " -> deleted, added as missing\n" ); fflush( stdout ); +#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 ); @@ -72,27 +68,39 @@ void T_FilesWatcher::check( ) // Handle missing/deleted files for ( auto it = missing_.begin( ) ; it < missing_.end( ) ; ++it ) { auto const& path( *it ); - //printf( "Checking missing file '%s'\n" , path.c_str( ) ); fflush( stdout ); +#ifdef INTRUSIVE_TRACES + printf( "Checking missing file '%s'\n" , path.toOSString( ).data( ) ); fflush( stdout ); +#endif // INTRUSIVE_TRACES auto* const p( fileWatchers_.get( path ) ); - //printf( " -> found? %c\n" , p != fileWatchers_.end( ) ? 'Y' : 'N' ); +#ifdef INTRUSIVE_TRACES + printf( " -> found? %c\n" , p ? 'Y' : 'N' ); +#endif // INTRUSIVE_TRACES assert( p ); - //printf( " -> wd = %d\n" , p->second.wd ); +#ifdef INTRUSIVE_TRACES + printf( " -> wd = %d\n" , p->wd ); +#endif // INTRUSIVE_TRACES assert( p->wd < 0 ); auto& we( *p ); const auto nwd( inotify_add_watch( fd , (char*) path.toOSString( ).data( ) , IN_CLOSE_WRITE | IN_DELETE_SELF ) ); - //printf( " -> inotify add watch %d\n" , nwd ); fflush( stdout ); +#ifdef INTRUSIVE_TRACES + printf( " -> inotify add watch %d\n" , nwd ); fflush( stdout ); +#endif // INTRUSIVE_TRACES if ( nwd < 0 ) { - //printf( " -> still missing\n" ); fflush( stdout ); +#ifdef INTRUSIVE_TRACES + printf( " -> still missing\n" ); fflush( stdout ); +#endif // INTRUSIVE_TRACES we.wd = -1; continue; } // Trigger if this was really a missing file if ( we.wd == -1 ) { - //printf( " -> triggering\n" ); fflush( stdout ); +#ifdef INTRUSIVE_TRACES + printf( " -> triggering\n" ); fflush( stdout ); +#endif // INTRUSIVE_TRACES we.trigger( ); } @@ -108,7 +116,7 @@ void T_FilesWatcher::check( ) void T_FilesWatcher::T_File_::trigger( ) { - for ( T_WatchedFiles* const wf : watchers ) { + for ( T_WatchSet_* const wf : watchers ) { if ( !wf->triggered ) { wf->triggered = true; wf->callback( ); @@ -118,12 +126,16 @@ void T_FilesWatcher::T_File_::trigger( ) void T_FilesWatcher::watchFile( T_String const& path , - T_WatchedFiles* const watcher ) + T_WatchSet_* const watcher ) { - //printf( "creating watch on '%s' for watcher %p\n" , path.c_str( ) , watcher ); fflush( stdout ); +#ifdef INTRUSIVE_TRACES + printf( "creating watch on '%s' for watcher %p\n" , path.toOSString( ).data( ) , watcher ); fflush( stdout ); +#endif // INTRUSIVE_TRACES auto* const exFilePos( fileWatchers_.get( path ) ); if ( exFilePos ) { - //printf( " -> existing\n" ); fflush( stdout ); +#ifdef INTRUSIVE_TRACES + printf( " -> existing\n" ); fflush( stdout ); +#endif // INTRUSIVE_TRACES auto& wl( exFilePos->watchers ); if ( !wl.contains( watcher ) ) { wl.add( watcher ); @@ -132,13 +144,19 @@ void T_FilesWatcher::watchFile( T_File_ f; f.wd = inotify_add_watch( fd , (char*) path.toOSString( ).data( ) , IN_CLOSE_WRITE | IN_DELETE_SELF ); - //printf( " -> inotify wd %d\n" , f.wd ); fflush( stdout ); +#ifdef INTRUSIVE_TRACES + printf( " -> inotify wd %d\n" , f.wd ); fflush( stdout ); +#endif // INTRUSIVE_TRACES if ( f.wd == -1 ) { missing_.add( path ); - //printf( " -> missing\n" ); fflush( stdout ); +#ifdef INTRUSIVE_TRACES + printf( " -> missing\n" ); fflush( stdout ); +#endif // INTRUSIVE_TRACES } else { inotify_.add( f.wd , path ); - //printf( " -> found\n" ); fflush( stdout ); +#ifdef INTRUSIVE_TRACES + printf( " -> found\n" ); fflush( stdout ); +#endif // INTRUSIVE_TRACES } f.watchers.add( watcher ); fileWatchers_.add( path , std::move( f ) ); @@ -146,7 +164,7 @@ void T_FilesWatcher::watchFile( } void T_FilesWatcher::unwatchAll( - T_WatchedFiles* const watcher ) + T_WatchSet_* const watcher ) { for ( auto i = 0u ; i < fileWatchers_.size( ) ; ) { auto const& path( fileWatchers_.keys( )[ i ] ); @@ -176,28 +194,17 @@ void T_FilesWatcher::unwatchAll( } } +/*----------------------------------------------------------------------------*/ -/*= T_WatchedFiles ===========================================================*/ - -T_WatchedFiles::T_WatchedFiles( T_WatchedFiles&& other ) noexcept - : watcher( other.watcher ) , callback( other.callback ) , - triggered( other.triggered ) +T_FilesWatcher::T_WatchSet_::T_WatchSet_( T_FilesWatcher* watcher , + F_OnFileChanges callback ) noexcept + : watcher( watcher ) , callback( std::move( callback ) ) , + triggered( false ) { - if ( watcher ) { - other.watcher = nullptr; - *( find( watcher->watched_ , &other ) ) = this; - } + watcher->watched_.add( this ); } -T_WatchedFiles::T_WatchedFiles( - T_FilesWatcher& watcher , - const F_OnFileChanges callback ) - : watcher( &watcher ) , callback( callback ) , triggered( false ) -{ - watcher.watched_.add( this ); -} - -T_WatchedFiles::~T_WatchedFiles( ) +T_FilesWatcher::T_WatchSet_::~T_WatchSet_( ) { if ( watcher ) { watcher->unwatchAll( this ); @@ -205,20 +212,32 @@ T_WatchedFiles::~T_WatchedFiles( ) } } +/*= T_WatchedFiles ===========================================================*/ + +T_WatchedFiles::T_WatchedFiles( T_WatchedFiles&& other ) noexcept + : watchSet_( std::move( other.watchSet_ ) ) +{ } + +T_WatchedFiles::T_WatchedFiles( + T_FilesWatcher& watcher , + F_OnFileChanges callback ) + : watchSet_( NewOwned< T_FilesWatcher::T_WatchSet_ >( &watcher , std::move( callback ) ) ) +{ } + void T_WatchedFiles::clear( ) { - if ( watcher ) { - watcher->unwatchAll( this ); + if ( watchSet_->watcher ) { + watchSet_->watcher->unwatchAll( watchSet_.get( ) ); } } bool T_WatchedFiles::watch( T_String const& file ) { - if ( !watcher ) { + if ( !watchSet_->watcher ) { return false; } - watcher->watchFile( file , this ); + watchSet_->watcher->watchFile( file , watchSet_.get( ) ); return true; } diff --git a/filewatcher.hh b/filewatcher.hh index de87294..b337f2e 100644 --- a/filewatcher.hh +++ b/filewatcher.hh @@ -22,17 +22,28 @@ struct T_FilesWatcher friend struct T_WatchedFiles; T_FilesWatcher( T_FilesWatcher const& ) = delete; + T_FilesWatcher( T_FilesWatcher&& ) = delete; T_FilesWatcher( ); - T_FilesWatcher( T_FilesWatcher&& ) noexcept; ~T_FilesWatcher( ); void check( ); private: + struct T_WatchSet_ { + T_FilesWatcher* watcher; + F_OnFileChanges callback; + bool triggered; + + T_WatchSet_( T_FilesWatcher* watcher , + F_OnFileChanges callback ) noexcept; + ~T_WatchSet_( ); + }; + using P_WatchSet_ = T_OwnPtr< T_WatchSet_ >; + struct T_File_ { int wd; - T_Array< T_WatchedFiles* > watchers; + T_Array< T_WatchSet_* > watchers; void trigger( ); }; @@ -40,11 +51,11 @@ struct T_FilesWatcher T_KeyValueTable< T_String , T_File_ > fileWatchers_; T_KeyValueTable< int , T_String > inotify_; T_Array< T_String > missing_; - T_Array< T_WatchedFiles* > watched_; + T_Array< T_WatchSet_* > watched_; void watchFile( T_String const& path , - T_WatchedFiles* const watcher ); - void unwatchAll( T_WatchedFiles* const watcher ); + T_WatchSet_* const watcher ); + void unwatchAll( T_WatchSet_* const watcher ); }; /*----------------------------------------------------------------------------*/ @@ -57,18 +68,13 @@ struct T_WatchedFiles T_WatchedFiles( T_WatchedFiles const& ) = delete; T_WatchedFiles( T_WatchedFiles&& ) noexcept; - T_WatchedFiles( - T_FilesWatcher& watcher , + T_WatchedFiles( T_FilesWatcher& watcher , const F_OnFileChanges callback ); - ~T_WatchedFiles( ); - void clear( ); bool watch( T_String const& file ); private: - T_FilesWatcher* watcher; - const F_OnFileChanges callback; - bool triggered; + T_FilesWatcher::P_WatchSet_ watchSet_; }; using P_WatchedFiles = T_OwnPtr< T_WatchedFiles >;