Re: [Bug Report + Patch] File descriptor leak when io_method=io_uring

From: Xuneng Zhou <xunengzhou(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Lucas DRAESCHER <git(at)draescher(dot)fr>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, Laetitia Avrot <laetitia(dot)avrot(at)gmail(dot)com>, "tanswis42(at)gmail(dot)com" <tanswis42(at)gmail(dot)com>, cca5507 <cca5507(at)qq(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [Bug Report + Patch] File descriptor leak when io_method=io_uring
Date: 2026-11-05 07:12:27
Message-ID: CABPTF7UvOU+-zUvb-1S2LgUX0-t1u54FdBhVks4MPx_12CtgDg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Fujii-san, Lucas,

On Tue, Nov 3, 2026 at 10:53 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> On Thu, Sep 10, 2026 at 3:10 AM Lucas DRAESCHER <git(at)draescher(dot)fr> wrote:
> > Alternatively, if maintaining a separate backpatch variant is undesirable,
> > v1 is still available. It doesn't change the `IoMethodOps` struct at all,
> > so the ABI question disappears.
>
> I don't object to the v4 approach for master, but I still prefer v1 for
> backpatching because it keeps the changes minimal and avoids changing
> the IoMethodOps ABI.

+1

> I've attached updated v1-based patches. The changes are:
> - Rename the cleanup function.
> - Replace the NULL check with an assertion and add Assert(!IsUnderPostmaster).
> - Remove the debug log message.
>
> considering a common cleanup interface separately for master if needed?

Thanks for the patch. It looks good overall. I tested
v1-2(max_connections=20, max_worker_processes=4, and
max_wal_senders=0). After a backend crash restart, the postmaster's
io_uring fd count of patched remained at 48, that of unpatched doubled
up to 96.

AI raised one concern about pgaio_uring_cleanup(). It passes
io_uring_queue_exit() a ring descriptor from shared memory which could
be corrupted by a backend before crashing. To reproduce this, it set
one ring's ring_fd to the postmaster's Unix listen fd, then killed the
backend. On restart, the postmaster closed the listening socket, which
refused further attempts to connect. Not sure about the possibility of
triggering memory bugs like this in reality. For cautiousness, do we
need to make a local copy of the ring ds when creation and use them
while cleanup? I see the pattern in sysv_sema.c, where the postmaster
keeps its own array of semaphore-set IDs for cleanup.

--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.

In response to

Browse pgsql-hackers by date

  From Date Subject
Previous Message Tatsuo Ishii 2026-11-05 07:11:17 Re: nodeWindowAgg.c IGNORE NULLS abs overflow