Permalink
Browse files

Stop leaving locks behind

Previously if some user cancelled his request (simply by pressing Stop button
in his browser), then the script will receive a TERM signal or the like.

This means that some locks could be left behind, which required someone
to unlock the wiki manually (by using Unlock Wiki action).

Now we remove these locks automatically!

However, some tasks might want to handle such situations gracefully. That's why
%LockCleaners hash was added. Use lock name as the key and put a coderef as a
value. If SIGTERM (or a bunch of other signals) is received, then it will run
this code, which, supposedly, cleans all of the stuff after it. Private Wiki
Extension was changed according to that, so you can see it in action.

Also, tests added!
  • Loading branch information...
AlexDaniel committed Sep 23, 2015
1 parent e77abbc commit 3b6d891dc7e5237ebfb0b6cc6f3001b888b91e87
Showing with 72 additions and 3 deletions.
  1. +4 −0 modules/private-wiki.pl
  2. +31 −1 t/lock.t
  3. +21 −0 t/test.pl
  4. +16 −2 wiki.pl
View
@@ -216,6 +216,7 @@ sub DoDiff { # Actualy call the diff program
my $oldName = "$TempDir/old";
my $newName = "$TempDir/new";
RequestLockDir('diff') or return '';
$LockCleaners{'diff'} = sub { unlink $oldName if -f $oldName; unlink $newName if -f $newName; };
OldPrivateWikiWriteStringToFile($oldName, $_[0]); # CHANGED Here we use the old sub!
OldPrivateWikiWriteStringToFile($newName, $_[1]); # CHANGED
my $diff_out = `diff -- \Q$oldName\E \Q$newName\E`;
@@ -235,6 +236,9 @@ sub MergeRevisions { # merge change from file2 to file3 into file1
my ($name1, $name2, $name3) = ("$TempDir/file1", "$TempDir/file2", "$TempDir/file3");
CreateDir($TempDir);
RequestLockDir('merge') or return T('Could not get a lock to merge!');
$LockCleaners{'merge'} = sub { # CHANGED
unlink $name1 if -f $name1; unlink $name2 if -f $name2; unlink $name3 if -f $name3;
};
OldPrivateWikiWriteStringToFile($name1, $file1); # CHANGED
OldPrivateWikiWriteStringToFile($name2, $file2); # CHANGED
OldPrivateWikiWriteStringToFile($name3, $file3); # CHANGED
View
@@ -1,3 +1,4 @@
# Copyright (C) 2015 Alex-Daniel Jakimenko <alex.jakimenko@gmail.com>
# Copyright (C) 2006 Alex Schroeder <alex@emacswiki.org>
#
# This program is free software; you can redistribute it and/or modify
@@ -18,7 +19,7 @@
require 't/test.pl';
package OddMuse;
use Test::More tests => 17;
use Test::More tests => 20;
test_page(get_page('action=editlock'), 'operation is restricted');
test_page(get_page('action=editlock pwd=foo'), 'Edit lock created');
@@ -36,3 +37,32 @@ test_page(update_page('TestLock', 'mu!'), 'This page does not exist');
test_page($redirect, 'Status: 503 SERVICE UNAVAILABLE',
'Could not get main lock', 'File exists',
'The lock was created \d+ seconds ago');
# Lock cleaners
AppendToConfig(<<'END');
$Action{'jobinterrupted'} = sub {
print GetHeader();
RequestLockDir('importantjob');
WriteStringToFile("$DataDir/deletemewhenfinished", 'bla-bla');
print 'Ok, doing some lengthy job... ';
sleep 15;
print 'Done!';
unlink "$DataDir/deletemewhenfinished"; # WHOOPS!
ReleaseLockDir('importantjob');
PrintFooter();
};
END
RunAndTerminate('perl', 'wiki.pl', 'action=jobinterrupted');
# first let's test that the action works (otherwise next test will give false "ok")
ok(-f "$DataDir/deletemewhenfinished", 'deletemewhenfinished file was created but not deleted');
ok(! -d "$TempDir/lockimportantjob", 'lock was deleted automatically');
AppendToConfig(<<'END');
$LockCleaners{'importantjob'} = sub {
unlink "$DataDir/deletemewhenfinished" if -f "$DataDir/deletemewhenfinished";
};
END
RunAndTerminate('perl', 'wiki.pl', 'action=jobinterrupted');
ok(! -f "${LockDir}deletemewhenfinished", 'Custom lock cleaning code works');
View
@@ -1,4 +1,5 @@
# Copyright (C) 2004–2015 Alex Schroeder <alex@gnu.org>
# Copyright (C) 2015 Alex-Daniel Jakimenko <alex.jakimenko@gmail.com>
#
# This program is free software; you can redistribute it and/or modify it under
# the terms of the GNU General Public License as published by the Free Software
@@ -349,4 +350,24 @@ sub clear_pages {
write_config_file();
}
sub RunAndTerminate { # runs a command for 1 second and then sends SIGTERM
my $pid = fork();
if (not $pid) { # child
open(STDOUT, '>', '/dev/null'); # we don't want to see the output
open(STDERR, '>', '/dev/null');
exec(@_) or die "Cannot start a new process: $!";
}
# parent
sleep 1;
kill 'TERM', $pid;
wait; # let it finish
}
sub AppendToConfig {
my @data = @_; # one or more strings
open(my $fh, '>>', "$DataDir/config") or die "Could not append to config file: $!";
print $fh join("\n", @data);
close $fh;
}
1;
View
18 wiki.pl
@@ -35,6 +35,7 @@ package OddMuse;
use CGI qw/-utf8/;
use CGI::Carp qw(fatalsToBrowser);
use File::Glob ':glob';
use sigtrap 'handler' => \&HandleSignals, 'normal-signals', 'error-signals';
local $| = 1; # Do not buffer output (localized for mod_perl)
# Options:
@@ -163,6 +164,7 @@ package OddMuse;
our @KnownLocks = qw(main diff index merge visitors); # locks to remove
our $LockExpiration = 60; # How long before expirable locks are expired
our %LockExpires = (diff=>1, index=>1, merge=>1, visitors=>1); # locks to expire after some time
our %LockCleaners = (); # What to do if a job under a lock gets a signal like SIGINT. e.g. 'diff' => \&CleanDiff
our %CookieParameters = (username=>'', pwd=>'', homepage=>'', theme=>'', css=>'', msg=>'', lang=>'', embed=>$EmbedWiki,
toplinkbar=>$TopLinkBar, topsearchform=>$TopSearchForm, matchingpages=>$MatchingPages, );
our %Action = (rc => \&BrowseRc, rollback => \&DoRollback,
@@ -2896,7 +2898,7 @@ sub RequestLockDir {
while (mkdir($lock, 0555) == 0) {
if ($n++ >= $tries) {
my $ts = (stat($lock))[9];
if ($Now - $ts > $LockExpiration and $LockExpires{$name} and not $retried) {
if ($Now - $ts > $LockExpiration and $LockExpires{$name} and not $retried) { # XXX should we remove this now?
ReleaseLockDir($name); # try to expire lock (no checking)
return 1 if RequestLockDir($name, undef, undef, undef, 1);
}
@@ -2915,6 +2917,18 @@ sub RequestLockDir {
return 1;
}
sub HandleSignals {
my ($signal) = @_; # TODO should we pass it to CleanLock?
CleanLock($_) foreach keys %Locks;
exit; # let's count it as graceful exit
}
sub CleanLock {
my ($name) = @_;
$LockCleaners{$name}->() if exists $LockCleaners{$name};
ReleaseLockDir($name); # TODO should we log this?
}
sub ReleaseLockDir {
my $name = shift; # We don't check whether we succeeded.
rmdir($LockDir . $name); # Before fixing, make sure we only call this
@@ -2935,7 +2949,7 @@ sub ForceReleaseLock {
foreach my $name (bsd_glob $pattern) {
# First try to obtain lock (in case of normal edit lock)
$forced = 1 unless RequestLockDir($name, 5, 3, 0);
ReleaseLockDir($name); # Release the lock, even if we didn't get it.
ReleaseLockDir($name); # Release the lock, even if we didn't get it. This should not happen.
}
return $forced;
}

0 comments on commit 3b6d891

Please sign in to comment.