Horton

Re: Using WaitEventSet in the postmaster

Thomas Munro <thomas.munro@gmail.com>

View thread View on postgr.es
From: Thomas Munro <thomas.munro@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2023-01-12T07:35:43Z
Lists: pgsql-hackers

Attachments

On Thu, Jan 12, 2023 at 7:57 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Thu, Jan 12, 2023 at 7:27 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > skink seems to have found a problem:
> >
> > ==2011873== VALGRINDERROR-BEGIN
> > ==2011873== Syscall param epoll_wait(events) points to unaddressable byte(s)
> > ==2011873==    at 0x4D8DC73: epoll_wait (epoll_wait.c:30)
> > ==2011873==    by 0x55CA49: WaitEventSetWaitBlock (latch.c:1527)
> > ==2011873==    by 0x55D591: WaitEventSetWait (latch.c:1473)
> > ==2011873==    by 0x4F2B28: ServerLoop (postmaster.c:1729)
> > ==2011873==    by 0x4F3E85: PostmasterMain (postmaster.c:1452)
> > ==2011873==    by 0x42643C: main (main.c:200)
> > ==2011873==  Address 0x7b1e620 is 1,360 bytes inside a recently re-allocated block of size 8,192 alloc'd
> > ==2011873==    at 0x48407B4: malloc (vg_replace_malloc.c:381)
> > ==2011873==    by 0x6D9D30: AllocSetContextCreateInternal (aset.c:446)
> > ==2011873==    by 0x4F2D9B: PostmasterMain (postmaster.c:614)
> > ==2011873==    by 0x42643C: main (main.c:200)
> > ==2011873==
> > ==2011873== VALGRINDERROR-END
>
> Repro'd here on Valgrind.  Oh, that's interesting.  WaitEventSetWait()
> wants to use an internal buffer of the size given to the constructor
> function, but passes the size of the caller's output buffer to
> epoll_wait() and friends.  Perhaps it should use Min(nevents,
> set->nevents_space).  I mean, I should have noticed that, but I think
> that's arguably a pre-existing bug in the WES code, or at least an
> unhelpful interface.  Thinking...

Yeah.  This stops valgrind complaining here.

Commits

  1. Remove unneeded volatile qualifiers from postmaster.c.

  2. Fix WaitEventSetWait() buffer overrun.

  3. Refactor DetermineSleepTime() to use milliseconds.

  4. Use WaitEventSet API for postmaster's event loop.

  5. Allow parent's WaitEventSets to be freed after fork().

  6. Don't leak a signalfd when using latches in the postmaster.

  7. Add WL_SOCKET_ACCEPT event to WaitEventSet API.

  8. From: Phil Thompson <phil@river-bank.demon.co.uk>