X-Git-Url: http://git.vanrenterghem.biz/git.ikiwiki.info.git/blobdiff_plain/1c7cdc5760f4096103cdb0fd99bee029bf8bc947..e59a948b44fa43f8c53833e590473c4185eaf782:/doc/bugs/locking_fun.mdwn diff --git a/doc/bugs/locking_fun.mdwn b/doc/bugs/locking_fun.mdwn index 8c4e0690b..46278b028 100644 --- a/doc/bugs/locking_fun.mdwn +++ b/doc/bugs/locking_fun.mdwn @@ -3,19 +3,103 @@ changes at once with its commit message (see r2779). The loser gets a message that there were conflicts and gets to see his own edits as the conflicting edits he's supposed to resolve. -This can happen because CGI.pm writes the change, then drops the lock -before calling rcs_commit. It can't keep the lock because the commit hook -needs to be able to lock. - -Using a shared reader lock plus an exclusive writer lock would seem to -allow getting around this. The CGI would need the exclusive lock when -editing the WC, then it could drop/convert that to the reader lock, keep -the lock open, and lauch the post-commit hook, which would use the reader -lock. - -One problem with the reader/writer idea is that the post-commit hook writes -wiki state. - -An alternative approach might be setting a flag that prevents the -post-commit hook from doing anything, and keeping the lock. Then the CGI -would do the render & etc that the post-commit hook normally does. +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. + +* As before, the CGI will take the main wiki lock when starting up. +* Before writing to the WC, the CGI takes an exclusive lock on the WC. +* After writing to the WC, the CGI can downgrade it to a shared lock. + (If this downgrade does not happen atomically, other CGIs can + steal the exclusive lock.) +* Then the CGI, as before, drops the main wiki lock to prevent deadlock. It + keeps its shared WC lock. +* The commit hook takes first the main wiki lock and then the shared WC lock + when starting up, and holds them until it's done. +* Once the commit is done, the CGI, as before, does not attempt to regain + the main wiki lock (that could deadlock). It does its final stuff and + exits, dropping the shared WC lock. + +Locking: + +Using fcntl locking from perl is very hard. flock locking has the problem +that one some OSes (linux?) converting an exclusive to a shared lock is not +atomic and can be raced. What happens if this race occurs is that, +since ikiwiki always uses LOCK_NB, the flock fails. Then we're back to the +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. + +------ + +My alternative idea, which seems simpler than all this tricky locking +stuff, is to introduce a new lock file (really a flag file implemented +using a lock), which tells the commit hook that the CGI is running, and +makes the commit hook a NOOP. + +* CGI takes the wikilock +* CGI writes changes to WC +* CGI sets wclock to disable the commit hook +* CGI does *not* drop the main wikilock +* CGI commit +* The commit hook tries to set the wclock, fails, and becomes 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 (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 +> of hook. The second case isn't a big deal if the CGI is gonna rerender +> everything anyhow. --[[Ethan]] + +I agree, and I think that the second case points to the hooks still being +responsible for sending out commit mails. Everything else the CGI can do. + +I don't believe that the first case is actually a problem: If the RCS +commit does not introduce a conflict then the CGI commit's changes will be +merged into the repo cleanly. OTOH, if the RCS commit does introduces a +conflict then the CGI commit will fail gracefully. This is exactly what +happens now if RCS commit happens while a CGI commit is in progress! Ie: + +* cgi takes the wikilock +* cgi writes change to wc +* svn commit -m "conflict" (this makes a change to repo immediately, then + runs the post-commit hook, which waits on the wikilock) +* cgi drops wikilock +* the post-commit hook from the above manual commit can now run. +* cgi calls rcs_commit, which fails due to the conflict just introduced + +The only difference to this scenario will be that the CGI will not drop the +wiki lock before its commit, and that the post-commit hook will turn into a +NOOP: + +* cgi takes the wikilock +* cgi writes change to wc +* cgi takes the wclock +* 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). +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]]