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 <[email protected]>
Reported-by: Noah Misch <[email protected]>
Discussion: https://postgr.es/m/20250324002939[email protected]

diff --git a/src/backend/storage/aio/aio_io.c b/src/backend/storage/aio/aio_io.c
index 36d2c1f492d251fb37970d04e5d24b967d3c3483..cc6d999a6fb2929bc62bf1ad6947f98d8e902277 100644 (file)
--- 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 2be6bb8972b0c56be82a3672add4bd859970dc9a..4a7853d13fac987efb12109cc23b2c8869185ff9 100644 (file)
--- 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
        {