From ef10416c6cdce0b2fbd7a0539c864655b18f0036 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Fri, 3 Jul 2020 16:19:06 +0530 Subject: [PATCH v1] Modify cancel_before_shmem_exit to search all registered exit callbacks cancel_before_shmem_exit: currently looks for only one entry pointed to by before_shmem_exit_index i.e last entry in before_shmem_exit_list array. This might have problems. For instance, (happens only postgres version 12) pg_start_backup makes nonexclusive_base_backup_cleanup callback entry in the list, executes a jit enabled query with jit compiler registering it's own exit call back llvm_shutdown, and then user issues pg_stop_backup which tries to unregister the exit callback i.e. nonexclusive_base_backup_cleanup using cancel_before_shmem_exit, looks at only the last element i.e. llvm_shutdown in the list and exits, causing intended one remaining in the list which gets executed at the proc exit of the session causing some of the callbacks not called at the intended moment. Hence, cancel_before_shmem_exit is imrpoved to look for the requested entry in the entire list/array not only the latest/last entry pointed to by before_shmem_exit_index and remove it. --- src/backend/storage/ipc/ipc.c | 61 ++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 8 deletions(-) diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c index bdbc2c3ac4..1475cf49df 100644 --- a/src/backend/storage/ipc/ipc.c +++ b/src/backend/storage/ipc/ipc.c @@ -381,19 +381,64 @@ on_shmem_exit(pg_on_exit_callback function, Datum arg) * cancel_before_shmem_exit * * this function removes a previously-registered before_shmem_exit - * callback. For simplicity, only the latest entry can be - * removed. (We could work harder but there is no need for - * current uses.) + * callback. Look through all entries in before_shmem_exit_list + * starting from the latest entry i.e. entry at + * before_shmem_exit_index - 1, try to find the entry, if found + * adjust the elements next to the found entry and decrement + * the before_shmem_exit_index. * ---------------------------------------------------------------- */ void cancel_before_shmem_exit(pg_on_exit_callback function, Datum arg) { - if (before_shmem_exit_index > 0 && - before_shmem_exit_list[before_shmem_exit_index - 1].function - == function && - before_shmem_exit_list[before_shmem_exit_index - 1].arg == arg) - --before_shmem_exit_index; + int idx; + + /* Start from the last entry. */ + idx = before_shmem_exit_index - 1; + + while (idx > 0) + { + if (before_shmem_exit_list[idx].function == function && + before_shmem_exit_list[idx].arg == arg) + { + /* Entry found. */ + if (idx == before_shmem_exit_index - 1) + { + /* + * Found entry is last entry in the array, + * so, just reinitialize function and arg, + * adjust the index and exit. + */ + before_shmem_exit_list[idx].function = NULL; + before_shmem_exit_list[idx].arg = (Datum) 0; + before_shmem_exit_index--; + break; + } + else + { + /* + * Found entry is not the last entry in the array, + * it is somewhere else in the array. Move the entries + * next to the found entry location + */ + int idx1 = idx; + + while (idx1 < before_shmem_exit_index - 1) + { + before_shmem_exit_list[idx1].function = before_shmem_exit_list[idx1+1].function; + before_shmem_exit_list[idx1].arg = before_shmem_exit_list[idx1+1].arg; + idx1++; + } + + /* Reinitialize the last entry, for further usage. */ + before_shmem_exit_list[idx1].function = NULL; + before_shmem_exit_list[idx1].arg = (Datum) 0; + before_shmem_exit_index--; + break; + } + } + idx--; + } } /* ---------------------------------------------------------------- -- 2.25.1