Re: Condition variables vs interrupts
Thomas Munro <thomas.munro@gmail.com>
From: Thomas Munro <thomas.munro@gmail.com>
To: Shawn Debnath <sdn@amazon.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>, Tomas Vondra <tomas.vondra@2ndquadrant.com>
Date: 2019-12-24T02:10:49Z
Lists: pgsql-hackers
Attachments
- 0001-Don-t-reset-latch-in-ConditionVariablePrepare-master.patch (application/octet-stream) patch 0001
- 0001-Don-t-reset-latch-in-ConditionVariableP-backbranches.patch (application/octet-stream) patch 0001
On Sat, Dec 21, 2019 at 2:10 PM Shawn Debnath <sdn@amazon.com> wrote: > On Fri, Dec 20, 2019 at 12:05:34PM +1300, Thomas Munro wrote: > > I think we should probably just remove the unusual ResetLatch() call, > > rather than adding a CFI(). See attached. Thoughts? > > I agree: removing the ResetLatch() and having the wait event code deal > with it is a better way to go. I wonder why the reset was done in the > first place? Thanks for the review. I was preparing to commit this today, but I think I've spotted another latch protocol problem in the stable branches only and I'd to get some more eyeballs on it. I bet it's really hard to hit, but ConditionVariableSleep()'s return path (ie when the CV has been signalled) forgets that the the latch is multiplexed and latches are merged: just because it was set by ConditionVariableSignal() doesn't mean it wasn't *also* set by die() or some other interrupt, and yet at the point we return, we've reset the latch and not run CFI(), and there's nothing to say the caller won't then immediately wait on the latch in some other code path, and sleep like a log despite the earlier delivery of (say) SIGTERM. If I'm right about that, I think the solution is to move that CFI down in the stable branches (which you already did for master in commit 1321509f).
Commits
-
Don't reset latch in ConditionVariablePrepareToSleep().
- 78aaa0e82335 13.0 landed
-
Introduce timed waits for condition variables.
- 1321509fa432 13.0 cited