Add a new 'generating_info_is_safe' cache option; if true, then process generating data (one that acquired exclusive writer's lock) would run 'generating_info' if there is no stale data and background cache generation is enabled. If function generating (or printing) exits, which leads to cache entry not being generated (for gitweb this means that there are pages which are not cached, i.e. error pages), and 'generating_info' also exits, this could result in bad behavior. Therefore this new option is false by default. Updates t9503 test appropriately. Signed-off-by: Jakub Narebski <jnareb@xxxxxxxxx> --- This patch was not present in previous version of this series. This is one solution to the problem mentioned in commit message; another is to use initial delay in "Generating..." page, as it is done in next patch, and as it was done in previous version of this series. gitweb/lib/GitwebCache/FileCacheWithLocking.pm | 17 ++++++++++++++--- t/t9503/test_cache_interface.pl | 1 + 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/gitweb/lib/GitwebCache/FileCacheWithLocking.pm b/gitweb/lib/GitwebCache/FileCacheWithLocking.pm index 694c318..0823c55 100644 --- a/gitweb/lib/GitwebCache/FileCacheWithLocking.pm +++ b/gitweb/lib/GitwebCache/FileCacheWithLocking.pm @@ -80,26 +80,35 @@ use POSIX qw(setsid); # instead), for other process (or bacground process). It is passed # $cache instance, $key, and opened $lock_fh filehandle to lockfile. # Unset by default (which means no activity indicator). +# * 'generating_info_is_safe' (boolean) +# If true, run 'generating_info' subroutine also in the project that +# is generating data. This has effect only when 'background_cache' +# is true (both 'background_cache' and 'generating_info_is_safe' must +# be true for project generating data to run 'generating_info'). +# Defaults to false for safety. sub new { my $class = shift; my %opts = ref $_[0] ? %{ $_[0] } : @_; my $self = $class->SUPER::new(\%opts); - my ($max_lifetime, $background_cache, $generating_info); + my ($max_lifetime, $background_cache, $generating_info, $gen_info_is_safe); if (%opts) { $max_lifetime = $opts{'max_lifetime'} || $opts{'max_cache_lifetime'}; $background_cache = $opts{'background_cache'}; $generating_info = $opts{'generating_info'}; + $gen_info_is_safe = $opts{'generating_info_is_safe'}; } $max_lifetime = -1 unless defined($max_lifetime); $background_cache = 1 unless defined($background_cache); + $gen_info_is_safe = 0 unless defined($gen_info_is_safe); $self->set_max_lifetime($max_lifetime); $self->set_background_cache($background_cache); $self->set_generating_info($generating_info); + $self->set_generating_info_is_safe($gen_info_is_safe); return $self; } @@ -110,7 +119,8 @@ sub new { # http://perldesignpatterns.com/perldesignpatterns.html#AccessorPattern # creates get_depth() and set_depth($depth) etc. methods -foreach my $i (qw(max_lifetime background_cache generating_info)) { +foreach my $i (qw(max_lifetime background_cache + generating_info generating_info_is_safe)) { my $field = $i; no strict 'refs'; *{"get_$field"} = sub { @@ -214,7 +224,8 @@ sub _set_maybe_background { # to regenerate/refresh the cache (generate data), # or if main process would show progress indicator $pid = fork() - if (@stale_result || $self->{'generating_info'}); + if (@stale_result || + ($self->{'generating_info'} && $self->{'generating_info_is_safe'})); } if ($pid) { diff --git a/t/t9503/test_cache_interface.pl b/t/t9503/test_cache_interface.pl index 81f0b76..480cfbc 100755 --- a/t/t9503/test_cache_interface.pl +++ b/t/t9503/test_cache_interface.pl @@ -333,6 +333,7 @@ subtest 'serving stale data when (re)generating' => sub { # with background generation $cache->set_background_cache(1); + $cache->set_generating_info_is_safe(1); $cache->set($key, $stale_value); $cache->set_expires_in(0); # set value is now expired -- 1.7.3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html