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

Hi,

Thanks for the report! I can reproduce this bug on master and your patch fixes it.

I've attached a patch which registers an on_shmem_exit() callback
to close the file descriptors on server exit. I took inspiration
from how src/backend/storage/aio/method_worker.c handles cleanup.

I also verify that only the postmaster will call this callback because all children
of the postmaster will reset postmaster's callback.

My another thought is that add a shmem_cleanup callback to IoMethodOps and
do cleanup in this callback. Not sure which is better.

--
Regards,
ChangAo Chen

Hi ChangAo,

Thanks for the review!

I've attached v2, which adds a shmem_cleanup callback to
IoMethodOps, registered in AioShmemInit().

Like you, I don't know which approach I prefer since the
new callback currently has only one implementor. If no
other IO method is expected to need cleanup, the simpler
v1 approach may be preferable. Happy to hear your thoughts.

I've also added this patch to the commitfest app:
https://commitfest.postgresql.org/patch/6617/

Regards,
Lucas.

I have attached v3 which rebases v2 on top of master.

v1 still applies cleanly.

Regards,

Lucas

The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not tested

Hi,

I looked at v3, focusing on the reported io_uring file descriptor leak across
backend crash restarts.

I used the v3 attachment from Lucas's 2026-04-17 message, on origin/master at
4cb2a9863d89b320f37eb1bd76822f6f65e59311.

The patch applies cleanly with git am, and git diff --check reports no issues.

I first did a non-liburing build on macOS:

./configure --prefix=.../pg-install --without-readline --without-zlib --without-icu
make -s -j8
make -s install

That passed. I could not run TAP tests locally there because IPC::Run is not
installed, and that build did not have liburing support.

I then tested on Linux 6.8.0 with liburing 2.5. I configured with:

./configure --prefix="$PWD/pg-install" --without-readline --without-zlib --without-icu --enable-tap-tests --with-liburing
make -s -j3
make -s install

configure found liburing and io_uring_queue_init_mem, and the build/install
passed.

The existing AIO TAP tests passed:

make -C src/test/modules/test_aio check

Result:

All tests successful.
Files=4, Tests=780
Result: PASS

For the reported leak, I compared unpatched master and v3 using an isolated
server configured with io_method=io_uring. In each cycle the script counted
the postmaster's /proc/<pid>/fd symlinks containing io_uring, started a client
backend running pg_sleep(), killed that backend with SIGKILL, waited for the
postmaster to reinitialize after the crash restart, and counted again.

On unpatched master:

initial_io_uring_fds=52
cycle=1 before=52 after=104 delta=52
cycle=2 before=104 after=156 delta=52
cycle=3 before=156 after=208 delta=52

With v3:

initial_io_uring_fds=52
cycle=1 before=52 after=52 delta=0
cycle=2 before=52 after=52 delta=0
cycle=3 before=52 after=52 delta=0

The v3 server log also showed the cleanup running at each reinitialization:

DEBUG: cleaning up 52 io_uring processes

So the reported leak reproduces for me on master, and v3 fixes it in this
reproducer.

I did not find a new issue in the checked path.

I have not reviewed older stable branches or the backpatching question, and I
did not run installcheck-world.

Regards,
Ilmar Yunusov

The new status of this patch is: Ready for Committer

Hi Lucas,

I reviewed v3 on Debian, kernel 6.17.13, liburing 2.14.

I reproduced the bug on unpatched master: fd count doubles from 142 to 284
after killing a backend. With the patch applied, the count stays stable.
The fix works.

Build was clean, no new warnings. All 245 regression tests pass, make
check-world passes, and all 39 TAP tests pass.

The code is clean and follows PostgreSQL conventions. The null guard before
registering the callback is correct, the wrapper satisfies the
on_shmem_exit() signature, and setting pgaio_uring_contexts to NULL after
cleanup prevents a double-free.

One note for the committer: on_shmem_exit() callbacks fire in LIFO order,
so this callback runs before anything registered earlier in AioShmemInit(),
which is the safe direction. Worth confirming that no earlier-registered
callback could invalidate pgaio_uring_contexts before this one runs.

On the v1 vs v2 design question: I favour v2. The Table AM precedent
suggests PostgreSQL prefers clean interface boundaries even with a single
implementor, and Andres Freund has been explicit about this in the past.

Marking as Ready for Committer.

Regards,

Lætitia

Le ven. 5 juin 2026 à 17:15, Lucas DRAESCHER <git@draescher.fr> a écrit :

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested

Hi Lucas,
I reviewed v3 on Debian, kernel 6.17.13, liburing 2.14.
I reproduced the bug on unpatched master: fd count doubles from 142 to 284 after killing a backend. With the patch applied, the count stays stable. The fix works.
Build was clean, no new warnings. All 245 regression tests pass, make check-world passes, and all 39 TAP tests pass.
The code is clean and follows PostgreSQL conventions. The null guard before registering the callback is correct, the wrapper satisfies the on_shmem_exit() signature, and setting pgaio_uring_contexts to NULL after cleanup prevents a double-free.
One note for the committer: on_shmem_exit() callbacks fire in LIFO order, so this callback runs before anything registered earlier in AioShmemInit(), which is the safe direction. Worth confirming that no earlier-registered callback could invalidate pgaio_uring_contexts before this one runs.
On the v1 vs v2 design question: I favour v2. The Table AM precedent suggests PostgreSQL prefers clean interface boundaries even with a single implementor, and Andres Freund has been explicit about this in the past.
Marking as Ready for Committer.
Regards,
Lætitia

Ilmar, Lætitia,

Thank you for taking the time to review my patch!

On the v1 vs v2 design question: I favour v2.
The Table AM precedent suggests PostgreSQL prefers
clean interface boundaries even with a single
implementor, and Andres Freund has been explicit
about this in the past.

Thanks for the feedback, I'm happy for v2 to be the
design we settle on.

Thanks again to both of you for your help.

Lucas.

On Wed, Jun 17, 2026 at 6:09 PM Lucas DRAESCHER <git@draescher.fr> wrote:

Thanks for the feedback, I'm happy for v2 to be the
design we settle on.

Could adding shmem_cleanup to IoMethodOps introduce an ABI break?
If so, the v1 approach seems preferable, at least for a backpatch to v18?

Regards,

--
Fujii Masao

On Fri, Jul 24, 2026 at 6:14 PM Fujii Masao <masao.fujii@gmail.com> wrote:

On Wed, Jun 17, 2026 at 6:09 PM Lucas DRAESCHER <git@draescher.fr> wrote:

Thanks for the feedback, I'm happy for v2 to be the
design we settle on.

I believe this needs some more attention as this ring fd leak is going to
cause harder outages in larger shops (single crash is going to cause leak,
but then double allocation of the rings due to ulimit causing errno/EMFILE)

$ export PGDATA=/tmp/pg20
$ cat /tmp/pg20/postgresql.auto.conf
io_method = 'io_uring'
max_connections = 1000 # even with smaller ones we are going to hit it anyway
$ grep 2042 logfile
HINT: Consider increasing "ulimit -n" to at least 2042.
$ ulimit -n
2042
$ ls -la "/proc/$(head -1 $PGDATA/postmaster.pid)/fd/" | grep "io_uring" | wc -l
1042
# simulate single dead backend (random bug)
$ kill -9 $(psql -h /tmp -XtA postgres -c "SELECT pid FROM
pg_stat_activity WHERE backend_type = 'client backend' LIMIT 1")
$ tail -7 logfile
LOG: client backend (PID 78247) was terminated by signal 9: Killed
LOG: terminating any other active server processes
LOG: all server processes terminated; reinitializing
FATAL: could not setup io_uring queue: Too many open files
HINT: Consider increasing "ulimit -n" to at least 2042.
LOG: database system is shut down <<<<<<<!!!!!

(but it is 2042 files, but we endup with way more than 2042)

I've analyzed it also from security PoV: the leaked fds are not being visible
to children (e.g. plperlu/COPY FROM PROGRAM/etc) as they were already marked
FD_CLOEXEC internally, so that's okay from that angle.

v2 LGTM to me, but does not apply due to AioShmemInit() taking some
args now and there is this confusion (v2 preferred and less prefered v3),
so maybe v4 should be sent to avoid confusion (v2 just rebased)?

Just some nit: shouldn't we return / Assert() if not IsUnderPostmaster in
pgaio_uring_shmem_cleanup()?

Could adding shmem_cleanup to IoMethodOps introduce an ABI break?
If so, the v1 approach seems preferable, at least for a backpatch to v18?

I'm not sure, but it seems we add it at the end of struct (?) also it's from
aio_internal and not exposed to extensions in any way (??)

-J.

Hi Lucas,

The CF entry is currently Ready for Committer, but the latest review
prefers v2 while noting that it no longer applies after the
AioShmemInit() change. It also leaves the IsUnderPostmaster check open.

Could you post a single vNext with the intended design, rebased to the
current API, and address that check? I have moved the entry to Waiting
on Author for now.

Thank you!

Best regards, Andrey Borodin.

Hi,

Thanks for your comments. Let me address them to the best of my ability.

I believe this needs some more attention as this ring fd leak is going to
cause harder outages in larger shops (single crash is going to cause leak,
but then double allocation of the rings due to ulimit causing errno/EMFILE)

This is actually how we encountered the bug in the first place, except we were
hitting `ulimit -l` since our servers have a very high `ulimit -n`:

```
FATAL: could not setup io_uring queue: Cannot allocate memory
LOG: database system is shut down
```

v2 LGTM to me, but does not apply due to AioShmemInit() taking some
args now and there is this confusion (v2 preferred and less prefered v3),
so maybe v4 should be sent to avoid confusion (v2 just rebased)?

I seem to have created some confusion with my versions, which I apologise
for. Let me try to correct that:
- v1: My initial naive implementation which adds the on_shmem_exit()
callback directly in pgaio_uring_shmem_init().
- v2: Adds the shmem_cleanup callback to IoMethodOps.
- v3: Rebases v2 because of the new API introduced in 58a1573.

So the rebase you ask for is v3, sent on the 17th of April. To settle the
numbering confusion I have attached v4, which I describe below.

Could adding shmem_cleanup to IoMethodOps introduce an ABI break?
If so, the v1 approach seems preferable, at least for a backpatch to v18?

Since aio_internal.h is installed as a server header and the new field is
added in the middle of the struct, v3 introduces an ABI break with PG 18
if backpatched.

As PG 18 is affected, I think this is worth backpatching. However, the new
shmem allocation functions aren't in PG 18, so the backpatch would need to
be v2-shaped and move `shmem_cleanup` to the end of the struct.

Since the backpatch needs a separate patch either way, master isn't constrained
here, so v4 keeps `shmem_cleanup` next to `shmem_callbacks`. I am happy to
send a backpatchable version if there is agreement on backpatching.

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.

Just some nit: shouldn't we return / Assert() if not IsUnderPostmaster in
pgaio_uring_shmem_cleanup()?

v4 adds an `Assert(!IsUnderPostmaster)` at the start of `pgaio_uring_shmem_cleanup`.

The CF entry is currently Ready for Committer, but the latest review
prefers v2 while noting that it no longer applies after the
AioShmemInit() change. It also leaves the IsUnderPostmaster check open.

Could you post a single vNext with the intended design, rebased to the
current API, and address that check? I have moved the entry to Waiting
on Author for now.

v4 addresses all these items. I have moved the CF entry to "Needs review".

Thanks again for all your feedback.

Lucas.