Fix bug of clearing of waitStart in ProcWakeup()
Hi,
I just noticed this while reviewing patch [1]/messages/by-id/CAHGQGwGw4LhNwOGQT3nbw3uWy8gL94_MB4T39Wfr4_Vgopuovg@mail.gmail.com. It looks like this is caused by a simple typo.
In ProcWakeup():
```
PGPROC *
ProcWakeup(PGPROC *proc, ProcWaitStatus waitStatus)
{
PGPROC *retProc;
/* Proc should be sleeping ... */
if (proc->links.prev == NULL ||
proc->links.next == NULL)
return NULL;
Assert(proc->waitStatus == PROC_WAIT_STATUS_WAITING);
/* Save next process before we zap the list link */
retProc = (PGPROC *) proc->links.next;
/* Remove process from wait queue */
SHMQueueDelete(&(proc->links));
(proc->waitLock->waitProcs.size)--;
/* Clean up process' state and pass it the ok/fail signal */
proc->waitLock = NULL;
proc->waitProcLock = NULL;
proc->waitStatus = waitStatus;
pg_atomic_write_u64(&MyProc->waitStart, 0); <== Here, it should operate on proc
/* And awaken it */
SetLatch(&proc->procLatch);
return retProc;
}
```
Since this function is clearly operating on the parameter proc, the only statement that touches MyProc looks suspicious. It should reset proc->waitStart, not MyProc->waitStart.
As written, it will incorrectly clear the caller’s waitStart instead of the awakened backend’s, potentially leaving a stale waitStart value in the target PGPROC.
[1]: /messages/by-id/CAHGQGwGw4LhNwOGQT3nbw3uWy8gL94_MB4T39Wfr4_Vgopuovg@mail.gmail.com
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Chao Li <li.evan.chao@gmail.com> 于2026年2月24日周二 14:29写道：
Hi,
I just noticed this while reviewing patch [1]. It looks like this is
caused by a simple typo.In ProcWakeup():
```
PGPROC *
ProcWakeup(PGPROC *proc, ProcWaitStatus waitStatus)
{
PGPROC *retProc;/* Proc should be sleeping ... */
if (proc->links.prev == NULL ||
proc->links.next == NULL)
return NULL;
Assert(proc->waitStatus == PROC_WAIT_STATUS_WAITING);/* Save next process before we zap the list link */
retProc = (PGPROC *) proc->links.next;/* Remove process from wait queue */
SHMQueueDelete(&(proc->links));
(proc->waitLock->waitProcs.size)--;/* Clean up process' state and pass it the ok/fail signal */
proc->waitLock = NULL;
proc->waitProcLock = NULL;
proc->waitStatus = waitStatus;
pg_atomic_write_u64(&MyProc->waitStart, 0); <== Here, it should
operate on proc/* And awaken it */
SetLatch(&proc->procLatch);return retProc;
}
```Since this function is clearly operating on the parameter proc, the only
statement that touches MyProc looks suspicious. It should reset
proc->waitStart, not MyProc->waitStart.As written, it will incorrectly clear the caller’s waitStart instead of
the awakened backend’s, potentially leaving a stale waitStart value in the
target PGPROC.[1]
/messages/by-id/CAHGQGwGw4LhNwOGQT3nbw3uWy8gL94_MB4T39Wfr4_Vgopuovg@mail.gmail.comBest regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Hi ,
The fix looks correct to me. I applied it locally and build and "make
check" passed from my side.
Regards,
ji xu
On Tue, Feb 24, 2026 at 4:31 PM ji xu <thanksgreed@gmail.com> wrote:
Chao Li <li.evan.chao@gmail.com> 于2026年2月24日周二 14:29写道：
Since this function is clearly operating on the parameter proc, the only statement that touches MyProc looks suspicious. It should reset proc->waitStart, not MyProc->waitStart.
As written, it will incorrectly clear the caller’s waitStart instead of the awakened backend’s, potentially leaving a stale waitStart value in the target PGPROC.
Thanks for the report!
You’re right. This leaves proc->waitStart unreset for a backend that has
woken up from a lock wait. In practice this doesn't seem to cause
user-visible issues, since pg_locks.waitstart is reported as NULL
when pg_locks.granted is true, regardless of proc->waitStart.
That said, the behavior is incorrect, so I'm feeling inclined to backpatch
a fix. Thoughts?
The fix looks correct to me. I applied it locally and build and "make check" passed from my side.
Sounds good to me.
Regards,
--
Fujii Masao
On 2026-Feb-24, ji xu wrote:
Chao Li <li.evan.chao@gmail.com> 于2026年2月24日周二 14:29写道：
```
PGPROC *
ProcWakeup(PGPROC *proc, ProcWaitStatus waitStatus)
{
pg_atomic_write_u64(&MyProc->waitStart, 0); <== Here, it should
operate on proc
I agree that this looks wrong. AFAICS it comes from 46d6e5f56790.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
[…] indem ich in meinem Leben oft an euch gedacht, euch glücklich zu machen. Seyd es!
A menudo he pensado en vosotros, en haceros felices. ¡Sedlo, pues!
Heiligenstädter Testament, L. v. Beethoven, 1802
https://de.wikisource.org/wiki/Heiligenstädter_Testament
On Wed, Feb 25, 2026 at 1:12 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:
On 2026-Feb-24, ji xu wrote:
Chao Li <li.evan.chao@gmail.com> 于2026年2月24日周二 14:29写道：
```
PGPROC *
ProcWakeup(PGPROC *proc, ProcWaitStatus waitStatus)
{pg_atomic_write_u64(&MyProc->waitStart, 0); <== Here, it should
operate on procI agree that this looks wrong. AFAICS it comes from 46d6e5f56790.
Yes, it's my mistake...
For the record, patch attached.
Regards,
--
Fujii Masao
Attachments:
v1-0001-Fix-ProcWakeup-resetting-wrong-waitStart-field.patchapplication/octet-stream; name=v1-0001-Fix-ProcWakeup-resetting-wrong-waitStart-field.patchDownload+1-2
On Feb 24, 2026, at 23:41, Fujii Masao <masao.fujii@gmail.com> wrote:
On Tue, Feb 24, 2026 at 4:31 PM ji xu <thanksgreed@gmail.com> wrote:
Chao Li <li.evan.chao@gmail.com> 于2026年2月24日周二 14:29写道：
Since this function is clearly operating on the parameter proc, the only statement that touches MyProc looks suspicious. It should reset proc->waitStart, not MyProc->waitStart.
As written, it will incorrectly clear the caller’s waitStart instead of the awakened backend’s, potentially leaving a stale waitStart value in the target PGPROC.
Thanks for the report!
I just realized that I forgot to attach the patch file. Anyway, that’s just a one-line change.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
v1-0001-Fix-bug-of-clearing-of-waitStart-in-ProcWakeup.patchapplication/octet-stream; name=v1-0001-Fix-bug-of-clearing-of-waitStart-in-ProcWakeup.patch; x-unix-mode=0644Download+1-2
On Wed, Feb 25, 2026 at 7:03 AM Chao Li <li.evan.chao@gmail.com> wrote:
On Feb 24, 2026, at 23:41, Fujii Masao <masao.fujii@gmail.com> wrote:
On Tue, Feb 24, 2026 at 4:31 PM ji xu <thanksgreed@gmail.com> wrote:
Chao Li <li.evan.chao@gmail.com> 于2026年2月24日周二 14:29写道：
Since this function is clearly operating on the parameter proc, the only statement that touches MyProc looks suspicious. It should reset proc->waitStart, not MyProc->waitStart.
As written, it will incorrectly clear the caller’s waitStart instead of the awakened backend’s, potentially leaving a stale waitStart value in the target PGPROC.
Thanks for the report!
I just realized that I forgot to attach the patch file. Anyway, that’s just a one-line change.
I've pushed the patch. Thanks!
Regards,
--
Fujii Masao
On Feb 26, 2026, at 08:08, Fujii Masao <masao.fujii@gmail.com> wrote:
On Wed, Feb 25, 2026 at 7:03 AM Chao Li <li.evan.chao@gmail.com> wrote:
On Feb 24, 2026, at 23:41, Fujii Masao <masao.fujii@gmail.com> wrote:
On Tue, Feb 24, 2026 at 4:31 PM ji xu <thanksgreed@gmail.com> wrote:
Chao Li <li.evan.chao@gmail.com> 于2026年2月24日周二 14:29写道：
Since this function is clearly operating on the parameter proc, the only statement that touches MyProc looks suspicious. It should reset proc->waitStart, not MyProc->waitStart.
As written, it will incorrectly clear the caller’s waitStart instead of the awakened backend’s, potentially leaving a stale waitStart value in the target PGPROC.
Thanks for the report!
I just realized that I forgot to attach the patch file. Anyway, that’s just a one-line change.
I've pushed the patch. Thanks!
Regards,
--
Fujii Masao
Hi Fujii-san, thank you very much for taking care of this patch.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/