Signed-off-by: Jakub Narebski <jnareb@xxxxxxxxx> --- This patch, as you can see, lack proper commit message: it is work in progress. Note that contrary to original patch by J.H. we fork unconditionally; otherwise the process which do recalculate cache entry would be at disadvantage compared to processes which failed race to acquire writers lock. Perhaps this should be made configurable, though. Also note that currently there is no limit on how stale the data which is being served while regenerating data can be, i.e. there is no equivalent for $maxCacheLife. Simplest solution (I guess) would be to just delete cache entry if it is too stale upfront. gitweb/cache.pm | 32 +++++++++++++++++++++++++++----- t/t9503/test_cache_interface.pl | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 5 deletions(-) diff --git a/gitweb/cache.pm b/gitweb/cache.pm index f514ee9..1adf467 100644 --- a/gitweb/cache.pm +++ b/gitweb/cache.pm @@ -426,14 +426,36 @@ sub compute { _Make_Path($lockfile); open my $lock_fh, '+>', $lockfile; # or die "Can't open lockfile '$lockfile': $!"; + + # try to retrieve stale data + $data = $self->restore($self->get_namespace(), $p_key); + if (my $lock_state = flock($lock_fh, LOCK_EX | LOCK_NB)) { # acquired writers lock - $data = $p_coderef->($self, $p_key); - $self->set($p_key, $data); + my $pid = fork() if $data; + if (!defined $pid || $pid) { + # parent, or didn't fork + $data = $p_coderef->($self, $p_key); + $self->set($p_key, $data); + + if ($pid) { + # wait for child (which would print) and exit + waitpid $pid, 0; + exit 0; + } else { + # there is no child, or was no $data to serve in background + ; + } + } else { + # child to serve $data + ; + } } else { - # get readers lock - flock($lock_fh, LOCK_SH); - $data = $self->restore($self->get_namespace(), $p_key); + if (!defined $data) { + # get readers lock if there is no stale data to serve + flock($lock_fh, LOCK_SH); + $data = $self->restore($self->get_namespace(), $p_key); + } } close $lock_fh; return $data; diff --git a/t/t9503/test_cache_interface.pl b/t/t9503/test_cache_interface.pl index 43b806d..788e8f2 100755 --- a/t/t9503/test_cache_interface.pl +++ b/t/t9503/test_cache_interface.pl @@ -154,5 +154,42 @@ is($cache->get_expires_in(), 0, '"expires in" is set to now (0)'); $cache->set($key, $value); ok(!defined($cache->get($key)), 'cache is expired'); +# Test that cache returns stale data in existing but expired cache situation +# (probably should be run only if GIT_TEST_LONG) +$cache->set_expires_min(0); +$cache->set_expires_max(0); +my $stale_value = 'Stale Value'; +my $child_data = ''; +$cache->set($key, $stale_value); +$call_count = 0; +$pid = open $kid_fh, '-|'; +SKIP: { + skip "cannot fork: $!", 4 + unless defined $pid; + + my $data = $cache->compute($key, \&get_value_slow); + + if ($pid) { + $child_data = <$kid_fh>; + chomp $child_data; + + waitpid $pid, 0; + close $kid_fh; + } else { + print "$data\n"; + exit 0; + } + + is($data, $stale_value, 'stale data in parent when expired'); + is($child_data, $stale_value, 'stale data in child when expired'); + + # never expire + $cache->set_expires_min(-1); + $cache->set_expires_max(-1); + is($cache->get($key), $value, 'value got set correctly'); +} +$cache->set_expires_min(0); +$cache->set_expires_max(0); + done_testing(); -- 1.6.6.1 -- 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