diff options
| -rw-r--r-- | doc/bugs/locking_fun.mdwn | 107 |
1 files changed, 10 insertions, 97 deletions
diff --git a/doc/bugs/locking_fun.mdwn b/doc/bugs/locking_fun.mdwn index 64d06917f..46278b028 100644 --- a/doc/bugs/locking_fun.mdwn +++ b/doc/bugs/locking_fun.mdwn @@ -7,6 +7,8 @@ This can happen because CGI.pm writes the change, then drops the main wiki lock before calling rcs_commit. It can't keep the lock because the commit hook needs to be able to lock. +------- + We batted this around for an hour or two on irc. The best solution seems to be adding a subsidiary second lock, which is only used to lock the working copy and is a blocking read/write lock. @@ -34,103 +36,7 @@ original race. It should be possible though to use a separate exclusive lock, wrapped around these flock calls, to force them to be "atomic" and avoid that race. -Sample patch, with stub functions for the new lock: - -[[toggle text="expand patch"]] -[[toggleable text=""" -<pre> -Index: IkiWiki/CGI.pm -=================================================================== ---- IkiWiki/CGI.pm (revision 2774) -+++ IkiWiki/CGI.pm (working copy) -@@ -494,9 +494,14 @@ - $content=~s/\r\n/\n/g; - $content=~s/\r/\n/g; - -+ lockwc_exclusive(); -+ - $config{cgi}=0; # avoid cgi error message - eval { writefile($file, $config{srcdir}, $content) }; - $config{cgi}=1; -+ -+ lockwc_shared(); -+ - if ($@) { - $form->field(name => "rcsinfo", value => rcs_prepedit($file), - force => 1); -Index: IkiWiki/Plugin/poll.pm -=================================================================== ---- IkiWiki/Plugin/poll.pm (revision 2770) -+++ IkiWiki/Plugin/poll.pm (working copy) -@@ -120,7 +120,9 @@ - $content =~ s{(\\?)\[\[poll\s+([^]]+)\s*\]\]}{$edit->($1, $2)}seg; - - # Store their vote, update the page, and redirect to it. -+ IkiWiki::lockwc_exclusive(); - writefile($pagesources{$page}, $config{srcdir}, $content); -+ IkiWiki::lockwc_shared(); - $session->param($choice_param, $choice); - IkiWiki::cgi_savesession($session); - $oldchoice=$session->param($choice_param); -@@ -130,6 +132,10 @@ - IkiWiki::rcs_commit($pagesources{$page}, "poll vote ($choice)", - IkiWiki::rcs_prepedit($pagesources{$page}), - $session->param("name"), $ENV{REMOTE_ADDR}); -+ # Make sure that the repo is up-to-date; -+ # locking prevents the post-commit hook -+ # from updating it. -+ rcs_update(); - } - else { - require IkiWiki::Render; -Index: ikiwiki.in -=================================================================== ---- ikiwiki.in (revision 2770) -+++ ikiwiki.in (working copy) -@@ -121,6 +121,7 @@ - lockwiki(); - loadindex(); - require IkiWiki::Render; -+ lockwc_shared(); - rcs_update(); - refresh(); - rcs_notify() if $config{notify}; -Index: IkiWiki.pm -=================================================================== ---- IkiWiki.pm (revision 2770) -+++ IkiWiki.pm (working copy) -@@ -617,6 +617,29 @@ - close WIKILOCK; - } #}}} - -+sub lockwc_exclusive () { #{{{ -+ # Take an exclusive lock on the working copy. -+ # The lock will be dropped on program exit. -+ # Note: This lock should only be taken _after_ the main wiki -+ # lock. -+ -+ # TODO -+} #}}} -+ -+sub lockwc_shared () { #{{{ -+ # Take a shared lock on the working copy. If an exclusive lock -+ # already exists, downgrade it to a shared lock. -+ # The lock will be dropped on program exit. -+ # Note: This lock should only be taken _after_ the main wiki -+ # lock. -+ -+ # TODO -+} #}}} -+ -+sub unlockwc () { #{{{ -+ close WIKIWCLOCK; -+} #}}} -+ - sub loadindex () { #{{{ - open (IN, "$config{wikistatedir}/index") || return; - while (<IN>) { -</pre> -"""]] +------ My alternative idea, which seems simpler than all this tricky locking stuff, is to introduce a new lock file (really a flag file implemented @@ -190,3 +96,10 @@ The commit hook takes them in the order (wclock, wikilock). This is a classic potential deadlock scenario. _However_, the commit hook should close the wclock as soon as it successfully opens it, before taking the wikilock, so I think that's ok. + +----- + +I've committed an implementation of my idea just above, and it seems to +work, although testing for races etc is tricky. Calling this [[bugs/done]] +unless someone finds a new bug or finds a problem in my thinking above. +--[[Joey]] |