doc/bugs/locking_fun.mdwn

   1 It's possible for concurrent web edits to race and the winner commits both
   2 changes at once with its commit message (see r2779). The loser gets a
   3 message that there were conflicts and gets to see his own edits as the
   4 conflicting edits he's supposed to resolve.
   5
   6 This can happen because CGI.pm writes the change, then drops the main wiki
   7 lock before calling rcs_commit. It can't keep the lock because the commit
   8 hook needs to be able to lock.
   9
  10 -------
  11
  12 We batted this around for an hour or two on irc. The best solution seems to
  13 be adding a subsidiary second lock, which is only used to lock the working
  14 copy and is a blocking read/write lock.
  15
  16 * As before, the CGI will take the main wiki lock when starting up.
  17 * Before writing to the WC, the CGI takes an exclusive lock on the WC.
  18 * After writing to the WC, the CGI can downgrade it to a shared lock.
  19   (If this downgrade does not happen atomically, other CGIs can
  20   steal the exclusive lock.)
  21 * Then the CGI, as before, drops the main wiki lock to prevent deadlock. It
  22   keeps its shared WC lock.
  23 * The commit hook takes first the main wiki lock and then the shared WC lock
  24   when starting up, and holds them until it's done.
  25 * Once the commit is done, the CGI, as before, does not attempt to regain
  26   the main wiki lock (that could deadlock). It does its final stuff and
  27   exits, dropping the shared WC lock.
  28
  29 Locking:
  30
  31 Using fcntl locking from perl is very hard. flock locking has the problem
  32 that one some OSes (linux?) converting an exclusive to a shared lock is not
  33 atomic and can be raced. What happens if this race occurs is that,
  34 since ikiwiki always uses LOCK_NB, the flock fails. Then we're back to the
  35 original race. It should be possible though to use a separate exclusive lock,
  36 wrapped around these flock calls, to force them to be "atomic" and avoid that
  37 race.
  38
  39 ------
  40
  41 My alternative idea, which seems simpler than all this tricky locking
  42 stuff, is to introduce a new lock file (really a flag file implemented
  43 using a lock), which tells the commit hook that the CGI is running, and
  44 makes the commit hook a NOOP.
  45
  46 * CGI takes the wikilock
  47 * CGI writes changes to WC
  48 * CGI sets wclock to disable the commit hook
  49 * CGI does *not* drop the main wikilock
  50 * CGI commit
  51 * The commit hook tries to set the wclock, fails, and becomes a noop
  52   (it may still need to send commit mails)
  53 * CGI removes wclock, thus re-enabling the commit hook
  54 * CGI updates the WC (since the commit hook didn't)
  55 * CGI renders the wiki (always. commits may have came in and not been
  56   rendered)
  57 * CGI checks for conflicts, and if any are found does its normal dance
  58
  59 > It seems like there are two things to be concerned with: RCS commit between
  60 > disable of hook and CGI commit, or RCS commit between CGI commit and re-enable
  61 > of hook. The second case isn't a big deal if the CGI is gonna rerender
  62 > everything anyhow. --[[Ethan]]
  63
  64 I agree, and I think that the second case points to the hooks still being
  65 responsible for sending out commit mails. Everything else the CGI can do.
  66
  67 I don't believe that the first case is actually a problem: If the RCS
  68 commit does not introduce a conflict then the CGI commit's changes will be
  69 merged into the repo cleanly. OTOH, if the RCS commit does introduces a
  70 conflict then the CGI commit will fail gracefully. This is exactly what
  71 happens now if RCS commit happens while a CGI commit is in progress! Ie:
  72
  73 * cgi takes the wikilock
  74 * cgi writes change to wc
  75 * svn commit -m "conflict" (this makes a change to repo immediately, then
  76   runs the post-commit hook, which waits on the wikilock)
  77 * cgi drops wikilock
  78 * the post-commit hook from the above manual commit can now run.
  79 * cgi calls rcs_commit, which fails due to the conflict just introduced
  80
  81 The only difference to this scenario will be that the CGI will not drop the
  82 wiki lock before its commit, and that the post-commit hook will turn into a
  83 NOOP:
  84
  85 * cgi takes the wikilock
  86 * cgi writes change to wc
  87 * cgi takes the wclock
  88 * svn commit -m "conflict" (this makes a change to repo immediately, then
  89   runs the post-commit hook, which becomes a NOOP)
  90 * cgi calls rcs_commit, which fails due to the conflict just introduced
  91 * cgi renders the wiki
  92
  93 Actually, the only thing that scares me about this apprach a little is that
  94 we have two locks. The CGI takes them in the order (wikilock, wclock).
  95 The commit hook takes them in the order (wclock, wikilock). This is a
  96 classic potential deadlock scenario. _However_, the commit hook should
  97 close the wclock as soon as it successfully opens it, before taking the
  98 wikilock, so I think that's ok.
  99
 100 -----
 101
 102 I've committed an implementation of my idea just above, and it seems to
 103 work, although testing for races etc is tricky. Calling this [[bugs/done]]
 104 unless someone finds a new bug or finds a problem in my thinking above.
 105 --[[Joey]]