Re: Possible race in UnlockBuffers() and UnpinBuffer() - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Possible race in UnlockBuffers() and UnpinBuffer()
Date
Msg-id 20875.1144945304@sss.pgh.pa.us
Whole thread Raw
In response to Possible race in UnlockBuffers() and UnpinBuffer()  ("Qingqing Zhou" <zhouqq@cs.toronto.edu>)
List pgsql-hackers
"Qingqing Zhou" <zhouqq@cs.toronto.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


pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: Practical impediment to supporting multiple SSL libraries
Next
From: Tom Lane
Date:
Subject: Re: Practical impediment to supporting multiple SSL libraries