Re: Possible race in UnlockBuffers() and UnpinBuffer()

"Qingqing Zhou" <zhouqq(at)cs(dot)toronto(dot)edu> writes:
> ... However, a possible execution sequence involving another process
> doing UnpinBuffer() may look like this:

> unpinner: lockHdr(); read and reset flag; unlockHdr();
> waiter: lockHdr(); reset flag; unlockHdr(); ProcCancelWaitForSignal();
> unpinner: ProcSendSignal();

Hmm ... I remember having convinced myself this code was OK, but I guess
I was wrong.

> After this, the proc->sem will be bumped to 1 unexpectedly ... Since this
> problem is rare, a possible fix is to put a critical section around line 1
> to 7 and remove UnlockBuffers() accordingly.

No, that would make any attempt to control-C a VACUUM have a significant
probability for panicking the whole database.

I think a better fix might be to arrange for an extra PGSemaphoreUnlock
to not be a problem. This is already true in lwlock.c, and in the
pin-count-waiter too (it'll just cause an extra cycle around the loop).
We'd have to modify ProcSleep to loop until it sees that someone has
actually granted or denied the lock, but that does not seem too hard.
(First thought about it is to change MyProc->waitStatus to have three
states, "waiting/ok/denied".) ProcCancelWaitForSignal could then go
away entirely.

regards, tom lane