Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix ProcWakeup() resetting wrong waitStart field.

  1. Fix bug of clearing of waitStart in ProcWakeup()

    Chao Li <li.evan.chao@gmail.com> — 2026-02-24T06:28:15Z

    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] https://postgr.es/m/CAHGQGwGw4LhNwOGQT3nbw3uWy8gL94_MB4T39Wfr4_Vgopuovg@mail.gmail.com
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
    
    
    
  2. Re: Fix bug of clearing of waitStart in ProcWakeup()

    ji xu <thanksgreed@gmail.com> — 2026-02-24T07:31:06Z

    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]
    > https://postgr.es/m/CAHGQGwGw4LhNwOGQT3nbw3uWy8gL94_MB4T39Wfr4_Vgopuovg@mail.gmail.com
    >
    > Best 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
    
  3. Re: Fix bug of clearing of waitStart in ProcWakeup()

    Fujii Masao <masao.fujii@gmail.com> — 2026-02-24T15:41:03Z

    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
    
    
    
    
  4. Re: Fix bug of clearing of waitStart in ProcWakeup()

    Álvaro Herrera <alvherre@kurilemu.de> — 2026-02-24T16:12:41Z

    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
    
    
    
    
  5. Re: Fix bug of clearing of waitStart in ProcWakeup()

    Fujii Masao <masao.fujii@gmail.com> — 2026-02-24T16:37:13Z

    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 proc
    >
    > I agree that this looks wrong.  AFAICS it comes from 46d6e5f56790.
    
    Yes, it's my mistake...
    
    For the record, patch attached.
    
    Regards,
    
    -- 
    Fujii Masao
    
  6. Re: Fix bug of clearing of waitStart in ProcWakeup()

    Chao Li <li.evan.chao@gmail.com> — 2026-02-24T22:03:14Z

    
    > 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/
    
    
    
    
    
  7. Re: Fix bug of clearing of waitStart in ProcWakeup()

    Fujii Masao <masao.fujii@gmail.com> — 2026-02-26T00:08:58Z

    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
    
    
    
    
  8. Re: Fix bug of clearing of waitStart in ProcWakeup()

    Chao Li <li.evan.chao@gmail.com> — 2026-02-26T01:04:01Z

    
    > 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/