Skip to content

Commit

Permalink
Adds new directories to watch *before* searching them for new entitie…
Browse files Browse the repository at this point in the history 
…s (GH#11)

When using `parse_events => 1` with inotify, there is a race condition
where entities in a new directory could be missed. We need to add the
new directory to the watch list before searching it for new files or
sub-directories.

Reported by @ufobat in GitHub Issue #11. Thanks!
  • Loading branch information
@mvgrimes
mvgrimes committed Dec 5, 2017
1 parent 2646b38 commit 8324180
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 51 deletions.
14 changes: 8 additions & 6 deletions lib/AnyEvent/Filesys/Notify.pm
View file
Expand Up @@ -42,24 +42,26 @@ sub _process_events {

# Some implementations provided enough information to parse the raw events,
# other require rescanning the file system (ie, Mac::FSEvents).
# The original behaviour was for rescan for all implementations, so we
# The original behavior was to rescan in all implementations, so we
# have added a flag to avoid breaking old code.

my @events;

if ( $self->parse_events and $self->can('_parse_events') ) {
@events = $self->_apply_filter( $self->_parse_events(@raw_events) );
@events =
$self->_parse_events( sub { $self->_apply_filter(@_) }, @raw_events);
} else {
my $new_fs = $self->_scan_fs( $self->dirs );
@events =
$self->_apply_filter( $self->_diff_fs( $self->_old_fs, $new_fs ) );
$self->_old_fs($new_fs);

# Some backends (when not using parse_events) need to add files
# (KQueue) or directories (Inotify2) to the watch list after they are
# created. Give them a chance to do that here.
$self->_post_process_events(@events) if $self->can('_post_process_events');
}

# Some backends need to add files (KQueue) or directories (Inotify2) to the
# watch list after they are created. Give them a chance to do that here.
$self->_process_events_for_backend(@events)
if $self->can('_process_events_for_backend');

$self->cb->(@events) if @events;

Expand Down
103 changes: 62 additions & 41 deletions lib/AnyEvent/Filesys/Notify/Role/Inotify2.pm
View file
Expand Up @@ -58,61 +58,82 @@ sub _init {
# Because of these differences, we default to the original behavior unless the
# parse_events flag is true.
sub _parse_events {
my ( $self, @raw_events ) = @_;
my @events = ();

for my $e (@raw_events) {
my $type = undef;

$type = 'modified' if ( $e->mask & ( IN_MODIFY | IN_ATTRIB ) );
$type = 'deleted' if ( $e->mask &
( IN_DELETE | IN_DELETE_SELF | IN_MOVED_FROM | IN_MOVE_SELF ) );
$type = 'created' if ( $e->mask & ( IN_CREATE | IN_MOVED_TO ) );

push(
@events,
AnyEvent::Filesys::Notify::Event->new(
path => $e->fullname,
type => $type,
is_dir => !! $e->IN_ISDIR,
) ) if $type;

# New directories are not automatically watched, we will add it to the
# list of watched directories in `around '_process_events'` but in
# the meantime, we will miss any newly created files in the subdir
if ( $e->IN_ISDIR and $type eq 'created' ) {
my $rule = Path::Iterator::Rule->new;
my $next = $rule->iter( $e->fullname );
while ( my $file = $next->() ) {
next if $file eq $e->fullname;
push @events,
AnyEvent::Filesys::Notify::Event->new(
path => $file,
type => 'created',
is_dir => -d $file,
);
}

}
my ( $self, $filter_cb, @raw_events ) = @_;

my @events =
map { $filter_cb->($_) } # filter new event
grep { defined } # filter undef events
map { $self->_mk_event($_) } @raw_events;

# New directories are not automatically watched by inotify.
$self->_add_events_to_watch(@events);

# Any entities that were created in new dirs, before the call to
# _add_events_to_watch, will be missed. So we walk the filesystem now.
push @events, map { $self->_add_entities_in_subdir( $filter_cb, $_ ) }
grep { $_->is_dir and $_->is_created } @events;

return @events;
}

sub _add_entities_in_subdir {
my ( $self, $filter_cb, $e ) = @_;
my @events;

my $rule = Path::Iterator::Rule->new;
my $next = $rule->iter( $e->path );
while ( my $file = $next->() ) {
next if $file eq $e->path;

my $new_event = AnyEvent::Filesys::Notify::Event->new(
path => $file,
type => 'created',
is_dir => -d $file,
);

next unless $filter_cb->($new_event);
push @events, $new_event;
}

return @events;
}

# Need to add newly created sub-dirs to the watch list.
# This is done after filtering. So entire dirs can be ignored efficiently;
sub _process_events_for_backend {
sub _mk_event {
my ( $self, $e ) = @_;

my $type = undef;

$type = 'modified' if ( $e->mask & ( IN_MODIFY | IN_ATTRIB ) );
$type = 'deleted'
if ( $e->mask &
( IN_DELETE | IN_DELETE_SELF | IN_MOVED_FROM | IN_MOVE_SELF ) );
$type = 'created' if ( $e->mask & ( IN_CREATE | IN_MOVED_TO ) );

return unless $type;
return AnyEvent::Filesys::Notify::Event->new(
path => $e->fullname,
type => $type,
is_dir => !!$e->IN_ISDIR,
);
}

sub _post_process_events {
my ( $self, @events ) = @_;
return $self->_add_events_to_watch(@events);
}

sub _add_events_to_watch {
my ( $self, @events ) = @_;

for my $event (@events) {
next unless $event->is_dir && $event->is_created;

print "adding to watch: " . $event->path . "\n";
$self->_fs_monitor->watch(
$event->path,
IN_MODIFY | IN_CREATE | IN_DELETE | IN_DELETE_SELF |
IN_MOVE | IN_MOVE_SELF | IN_ATTRIB,
IN_MOVE | IN_MOVE_SELF | IN_ATTRIB,
sub { my $e = shift; $self->_process_events($e); } );

}
}

Expand Down
9 changes: 5 additions & 4 deletions lib/AnyEvent/Filesys/Notify/Role/KQueue.pm
View file
Expand Up @@ -46,10 +46,11 @@ sub _init {
return 1;
}

# Need to add newly created items (directories and files) or remove deleted items.
# This isn't going to be perfect. If the path is not canonical then we won't deleted it.
# This is done after filtering. So entire dirs can be ignored efficiently.
sub _process_events_for_backend {
# Need to add newly created items (directories and files) or remove deleted
# items. This isn't going to be perfect. If the path is not canonical then we
# won't deleted it. This is done after filtering. So entire dirs can be
# ignored efficiently.
sub _post_process_events {
my ( $self, @events ) = @_;

for my $event (@events) {
Expand Down
45 changes: 45 additions & 0 deletions t/42-inotify-race.t
View file
@@ -0,0 +1,45 @@
use Test::More;

# GitHub issue #11
# Previous implementation had a race condition which could miss entities
# created inside a newly create directory.

use strict;
use warnings;
use File::Spec;
use lib 't/lib';
$|++;

use TestSupport qw(create_test_files delete_test_files move_test_files
modify_attrs_on_test_files $dir received_events receive_event);

use AnyEvent::Filesys::Notify;
use AnyEvent::Impl::Perl;

create_test_files(qw(one/1 two/1));
## ls: one/1 two/1

my $n = AnyEvent::Filesys::Notify->new(
dirs => [ map { File::Spec->catfile( $dir, $_ ) } qw(one two) ],
filter => sub { shift !~ qr/ignoreme/ },
cb => sub { receive_event(@_) },
parse_events => 1,
);
isa_ok( $n, 'AnyEvent::Filesys::Notify' );

diag "This might take a few seconds to run...";

received_events( sub { create_test_files(qw(one/sub/2)) },
'create subdir and file', qw(created created) );
## ls: one/sub/1 one/sub/2 two/1

received_events( sub { create_test_files(qw(one/sub/ignoreme/1 one/sub/3)) },
'create subdir and file', qw(created) );
## ls: one/sub/1 one/sub/2 one/sub/ignoreme/1 one/sub/3 two/1

received_events( sub { create_test_files(qw(two/sub/ignoreme/sub/1)) },
'create subdir and file', qw(created) );
## ls: one/sub/1 one/sub/2 one/sub/ignoreme/1 one/sub/3 two/1 tow/sub/ignoreme/sub/1

## ok( 1, '... arrived' );
done_testing;

0 comments on commit 8324180

Please sign in to comment.