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.
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
(it may still need to send commit mails)
* CGI removes wclock, thus re-enabling the commit hook
* CGI updates the WC (since the commit hook didn't)
-* CGI renders the wiki
+* CGI renders the wiki (always. commits may have came in and not been
+ rendered)
+* CGI checks for conflicts, and if any are found does its normal dance
> It seems like there are two things to be concerned with: RCS commit between
> disable of hook and CGI commit, or RCS commit between CGI commit and re-enable
* svn commit -m "conflict" (this makes a change to repo immediately, then
runs the post-commit hook, which becomes a NOOP)
* cgi calls rcs_commit, which fails due to the conflict just introduced
+* cgi renders the wiki
Actually, the only thing that scares me about this apprach a little is that
we have two locks. The CGI takes them in the order (wikilock, wclock).
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]]