From 96da9050a57aece4a48ab34a84bc3b3412708a20 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 26 Mar 2025 16:06:54 -0400 Subject: [PATCH] aio: Be more paranoid about interrupts As reported by Noah, it's possible, although practically very unlikely, that interrupts could be processed in between pgaio_io_reopen() and pgaio_io_perform_synchronously(). Prevent that by explicitly holding interrupts. It also seems good to add an assertion to pgaio_io_before_prep() to ensure that interrupts are held, as otherwise FDs referenced by the IO could be closed during interrupt processing. All code in the aio series currently runs the code with interrupts held, but it seems better to be paranoid. Reviewed-by: Noah Misch Reported-by: Noah Misch Discussion: https://postgr.es/m/20250324002939.5c.nmisch@google.com --- src/backend/storage/aio/aio_io.c | 6 ++++++ src/backend/storage/aio/method_worker.c | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/src/backend/storage/aio/aio_io.c b/src/backend/storage/aio/aio_io.c index 36d2c1f492d..cc6d999a6fb 100644 --- a/src/backend/storage/aio/aio_io.c +++ b/src/backend/storage/aio/aio_io.c @@ -159,6 +159,12 @@ pgaio_io_before_prep(PgAioHandle *ioh) Assert(pgaio_my_backend->handed_out_io == ioh); Assert(pgaio_io_has_target(ioh)); Assert(ioh->op == PGAIO_OP_INVALID); + + /* + * Otherwise the FDs referenced by the IO could be closed due to interrupt + * processing. + */ + Assert(!INTERRUPTS_CAN_BE_PROCESSED()); } /* diff --git a/src/backend/storage/aio/method_worker.c b/src/backend/storage/aio/method_worker.c index 2be6bb8972b..4a7853d13fa 100644 --- a/src/backend/storage/aio/method_worker.c +++ b/src/backend/storage/aio/method_worker.c @@ -476,6 +476,13 @@ IoWorkerMain(const void *startup_data, size_t startup_data_len) "worker %d processing IO", MyIoWorkerId); + /* + * Prevent interrupts between pgaio_io_reopen() and + * pgaio_io_perform_synchronously() that otherwise could lead to + * the FD getting closed in that window. + */ + HOLD_INTERRUPTS(); + /* * It's very unlikely, but possible, that reopen fails. E.g. due * to memory allocations failing or file permissions changing or @@ -502,6 +509,8 @@ IoWorkerMain(const void *startup_data, size_t startup_data_len) * ensure we don't accidentally fail. */ pgaio_io_perform_synchronously(ioh); + + RESUME_INTERRUPTS(); } else {