On Wed, Jan 27, 2021 at 9:34 AM Thomas Munro <thomas.mu...@gmail.com> wrote: > On Wed, Jan 27, 2021 at 12:22 AM Kyotaro Horiguchi > <horikyota....@gmail.com> wrote: > > At Tue, 26 Jan 2021 11:00:56 +0200, Heikki Linnakangas <hlinn...@iki.fi> > > wrote in > > > Don't we potentially have the same problem with all on_dsm_detach > > > callbacks? Looking at the other on_dsm_detach callbacks, I don't see > > > any CHECK_FOR_INTERRUPT() calls in them, but it seems fragile to > > > assume that. > > > > > > I'd suggest adding HOLD/RESUME_INTERRUPTS() to dsm_detach(). At least > > > around the removal of the callback from the list and calling the > > > callback. Maybe even over the whole function. > > > > Yes, I first came up with HOLD/RESUME_QUERY_INTERRUPTS() to the same > > location. > > +1, this seems like a good idea. This is a little bit like the code > near the comments "Don't joggle the elbow of proc_exit".
So that gives a very simple back-patchable patch.
From b27cbabee9a5980c8673c4fee4ea6f7e0c89bdbc Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Sun, 31 Jan 2021 14:04:02 +1300 Subject: [PATCH] Hold interrupts while running dsm_detach() callbacks. While cleaning up after a parallel query or parallel index creation that created temporary files, we could be interrupted by a statement timeout. The error handling path would then fail to clean up the files too, because the callback was already popped off the list. Prevent this hazard by holding interrupts while the cleanup code runs. Thanks to Heikki Linnakangas for this suggestion. Back-patch to all supported releases. Reported-by: Justin Pryzby <pry...@telsasoft.com> Discussion: https://postgr.es/m/20191212180506.gr2...@telsasoft.com --- src/backend/storage/ipc/dsm.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c index ae82b4bdc0..23bf192727 100644 --- a/src/backend/storage/ipc/dsm.c +++ b/src/backend/storage/ipc/dsm.c @@ -771,8 +771,10 @@ dsm_detach(dsm_segment *seg) /* * Invoke registered callbacks. Just in case one of those callbacks * throws a further error that brings us back here, pop the callback - * before invoking it, to avoid infinite error recursion. + * before invoking it, to avoid infinite error recursion. Don't allow + * interrupts to prevent cleanup from running to completion. */ + HOLD_INTERRUPTS(); while (!slist_is_empty(&seg->on_detach)) { slist_node *node; @@ -788,6 +790,7 @@ dsm_detach(dsm_segment *seg) function(seg, arg); } + RESUME_INTERRUPTS(); /* * Try to remove the mapping, if one exists. Normally, there will be, but -- 2.20.1
