X-Git-Url: http://git.vanrenterghem.biz/git.ikiwiki.info.git/blobdiff_plain/9d48f4c702cff8ab2621676269dc75535d748bd4..6975f9e3f4e06fb55cc8c9795c503019d2e8d08b:/doc/bugs/locking_fun.mdwn?ds=inline diff --git a/doc/bugs/locking_fun.mdwn b/doc/bugs/locking_fun.mdwn index 6d5f79ce5..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=""" -
-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 () {
-
-"""]] +------ My alternative idea, which seems simpler than all this tricky locking stuff, is to introduce a new lock file (really a flag file implemented @@ -146,7 +52,9 @@ makes the commit hook a NOOP. (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 @@ -180,6 +88,7 @@ NOOP: * 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). @@ -187,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]]