diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index 97f3cb625e2..a90dba0662f 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3740,7 +3740,10 @@ my_shmem_init(void *arg) on whether the requested memory areas were already initialized by another backend. The callbacks will be called while holding an internal lock (ShmemIndexLock), which prevents the race condition of two backends - trying to initialize the memory area at the same time. + trying to initialize the memory area at the same time. If the + allocation or initialization fails for any reason, the shared memory + areas are left in an abandoned state and any attempt to attach or + re-initialize them will fail until the server is restarted. diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c index f88642e6b8b..f971ee24192 100644 --- a/src/backend/storage/ipc/shmem.c +++ b/src/backend/storage/ipc/shmem.c @@ -144,6 +144,8 @@ #include "utils/builtins.h" #include "utils/tuplestore.h" +typedef struct ShmemIndexEnt ShmemIndexEnt; + /* * Registered callbacks. * @@ -166,6 +168,9 @@ typedef struct { ShmemStructOpts *options; ShmemRequestKind kind; + + /* InitShmemIndexEntry() sets this pointer when the area is allocated */ + ShmemIndexEnt *index_entry; } ShmemRequest; static List *pending_shmem_requests; /* List of ShmemRequests */ @@ -264,12 +269,13 @@ static HTAB *ShmemIndex; #define SHMEM_INDEX_ADDITIONAL_SIZE (128) /* this is a hash bucket in the shmem index table */ -typedef struct +typedef struct ShmemIndexEnt { char key[SHMEM_INDEX_KEYSIZE]; /* string name */ void *location; /* location in shared mem */ Size size; /* # bytes requested for the structure */ Size allocated_size; /* # bytes actually allocated */ + bool initialized; /* has the init callback been run? */ } ShmemIndexEnt; /* To get reliable results for NUMA inquiry we need to "touch pages" once */ @@ -382,6 +388,7 @@ ShmemRequestInternal(ShmemStructOpts *options, ShmemRequestKind kind) request = palloc_object(ShmemRequest); request->options = options; request->kind = kind; + request->index_entry = NULL; pending_shmem_requests = lappend(pending_shmem_requests, request); MemoryContextSwitchTo(oldcontext); } @@ -441,10 +448,7 @@ ShmemInitRequested(void) foreach_ptr(ShmemRequest, request, pending_shmem_requests) { InitShmemIndexEntry(request); - pfree(request->options); } - list_free_deep(pending_shmem_requests); - pending_shmem_requests = NIL; /* * Call the subsystem-specific init callbacks to finish initialization of @@ -456,6 +460,15 @@ ShmemInitRequested(void) callbacks->init_fn(callbacks->opaque_arg); } + /* Now we can mark all the areas as initialized and free the requests */ + foreach_ptr(ShmemRequest, request, pending_shmem_requests) + { + request->index_entry->initialized = true; + pfree(request->options); + } + list_free_deep(pending_shmem_requests); + pending_shmem_requests = NIL; + shmem_request_state = SRS_DONE; } @@ -557,7 +570,12 @@ InitShmemIndexEntry(ShmemRequest *request) index_entry->allocated_size = allocated_size; index_entry->location = structPtr; - /* Initialize depending on the kind of shmem area it is */ + /* + * The area is considered fully initialized only after the subsystem's + * init callback has been called. For now, perform only basic + * initialization based on the kind of shmem area it is. + */ + index_entry->initialized = false; switch (request->kind) { case SHMEM_KIND_STRUCT: @@ -571,6 +589,9 @@ InitShmemIndexEntry(ShmemRequest *request) shmem_slru_init(structPtr, request->options); break; } + + /* return the pointer to the entry to the caller */ + request->index_entry = index_entry; } /* @@ -600,6 +621,20 @@ AttachShmemIndexEntry(ShmemRequest *request, bool missing_ok) return false; } + /* + * If it was previously allocated but not fully initialized, error out. + * There is currently no way of retrying or cleaning up an uninitialized + * entry, it just lingers until the server is shut down. But this can + * only happen when allocating areas after postmaster startup, and it's + * unlikely that you could successfully retry anyway. The most likely + * reason for failed initialization is that you are out of shared memory + * and retrying won't help with that. + */ + if (!index_entry->initialized) + ereport(ERROR, + (errmsg("cannot attach to shared memory struct \"%s\" because it was not fully initialized", + request->options->name))); + /* Check that the size in the index matches the request */ if (index_entry->size != request->options->size && request->options->size != SHMEM_ATTACH_UNKNOWN_SIZE) @@ -628,6 +663,8 @@ AttachShmemIndexEntry(ShmemRequest *request, bool missing_ok) break; } + request->index_entry = index_entry; + return true; } @@ -738,6 +775,7 @@ InitShmemAllocator(PGShmemHeader *seghdr) result->size = ShmemAllocator->index_size; result->allocated_size = ShmemAllocator->index_size; result->location = ShmemAllocator->index; + result->initialized = true; } } @@ -974,7 +1012,17 @@ ProcessShmemRequestsAfterStartup(const ShmemCallbacks *callbacks) index_entry = (ShmemIndexEnt *) hash_search(ShmemIndex, request->options->name, HASH_FIND, NULL); if (index_entry) + { + /* + * Check for a half-initialized area. (See also similar check in + * AttachShmemIndexEntry()) + */ + if (!index_entry->initialized) + ereport(ERROR, + (errmsg("cannot attach to shared memory struct \"%s\" because it was not fully initialized", + request->options->name))); found_any = true; + } else notfound_any = true; } @@ -1005,6 +1053,11 @@ ProcessShmemRequestsAfterStartup(const ShmemCallbacks *callbacks) callbacks->init_fn(callbacks->opaque_arg); } + foreach_ptr(ShmemRequest, request, pending_shmem_requests) + { + request->index_entry->initialized = true; + } + LWLockRelease(ShmemIndexLock); } @@ -1069,7 +1122,11 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr) /* Initialize it if not found */ if (!*foundPtr) + { InitShmemIndexEntry(&request); + /* no additional initialization needed */ + request.index_entry->initialized = true; + } LWLockRelease(ShmemIndexLock); diff --git a/src/test/modules/test_shmem/Makefile b/src/test/modules/test_shmem/Makefile index 2407f7462fe..fed8e29c8f5 100644 --- a/src/test/modules/test_shmem/Makefile +++ b/src/test/modules/test_shmem/Makefile @@ -2,6 +2,9 @@ PGFILEDESC = "test_shmem - test code for shmem allocations" +EXTRA_INSTALL = src/test/modules/injection_points +export enable_injection_points + MODULE_big = test_shmem OBJS = \ $(WIN32RES) \ diff --git a/src/test/modules/test_shmem/meson.build b/src/test/modules/test_shmem/meson.build index fb4bf328b8f..8f98f2c4e31 100644 --- a/src/test/modules/test_shmem/meson.build +++ b/src/test/modules/test_shmem/meson.build @@ -26,6 +26,9 @@ tests += { 'sd': meson.current_source_dir(), 'bd': meson.current_build_dir(), 'tap': { + 'env': { + 'enable_injection_points': get_option('injection_points') ? 'yes' : 'no', + }, 'tests': [ 't/001_late_shmem_alloc.pl', ], diff --git a/src/test/modules/test_shmem/t/001_late_shmem_alloc.pl b/src/test/modules/test_shmem/t/001_late_shmem_alloc.pl index 6ea409f3c63..a7126ebce3f 100644 --- a/src/test/modules/test_shmem/t/001_late_shmem_alloc.pl +++ b/src/test/modules/test_shmem/t/001_late_shmem_alloc.pl @@ -7,17 +7,20 @@ use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; +# Initialize a cluster with the extension installed. The tests will +# call the function that comes with the extension to load it. +my $node = PostgreSQL::Test::Cluster->new('main'); +$node->init; +$node->start; +$node->safe_psql("postgres", "CREATE EXTENSION test_shmem"); +$node->stop; + ### # Test allocating memory after startup, i.e. when the library is not # in shared_preload_libraries ### -my $node = PostgreSQL::Test::Cluster->new('main'); -$node->init; $node->start; - -$node->safe_psql("postgres", "CREATE EXTENSION test_shmem;"); - # Check that the attach counter is incremented on a new connection my $attach_count1 = $node->safe_psql("postgres", "SELECT get_test_shmem_attach_count();"); @@ -25,6 +28,7 @@ my $attach_count2 = $node->safe_psql("postgres", "SELECT get_test_shmem_attach_count();"); cmp_ok($attach_count2, '>', $attach_count1, "attach callback is called in each backend"); + $node->stop; ### @@ -82,6 +86,42 @@ else $node->stop; $node->adjust_conf('postgresql.conf', "shared_preload_libraries", undef); +### +# Test a failure in initializing the shared memory area +### +SKIP: +{ + skip "injection points not supported by this build", + if $ENV{enable_injection_points} ne 'yes'; + $node->start; + $node->safe_psql("postgres", "CREATE EXTENSION injection_points;"); + $node->safe_psql("postgres", + "SELECT injection_points_attach('test-shmem-init', 'error');"); + + # Try to load the extension library. It will hit the injected + # error in the init callback. + my (undef, undef, $stderr) = + $node->psql("postgres", "SELECT get_test_shmem_attach_count();"); + like( + $stderr, + qr/error triggered for injection point test-shmem-init/, + "failure in initialization is reported"); + $node->safe_psql("postgres", + "SELECT injection_points_detach('test-shmem-init');"); + + # The error leaves the shared memory area in a broken state. + # Attempting to initialize or attach it again will fail, until the + # server is restarted. + (undef, undef, $stderr) = + $node->psql("postgres", "SELECT get_test_shmem_attach_count();"); + like( + $stderr, + qr/cannot attach to shared memory/, + "post-init extension creation fails"); + + $node->stop; +} + ### # Test "out of shared memory" in an after-startup request ### diff --git a/src/test/modules/test_shmem/test_shmem.c b/src/test/modules/test_shmem/test_shmem.c index 231ad9a0027..6cf47dc8968 100644 --- a/src/test/modules/test_shmem/test_shmem.c +++ b/src/test/modules/test_shmem/test_shmem.c @@ -68,6 +68,9 @@ static void test_shmem_init(void *arg) { elog(LOG, "init callback called"); + + INJECTION_POINT("test-shmem-init", NULL); + if (TestShmem->initialized) elog(ERROR, "shmem area already initialized"); TestShmem->initialized = true;