From 77acab75dfe2e4741c25c0cf550266caef1eebd2 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 28 Apr 2010 16:54:16 +0000 Subject: [PATCH] Modify ShmemInitStruct and ShmemInitHash to throw errors internally, rather than returning NULL for some-but-not-all failures as they used to. Remove now-redundant tests for NULL from call sites. We had to do something about this because many call sites were failing to check for NULL; and changing it like this seems a lot more useful and mistake-proof than adding checks to the call sites without them. --- .../pg_stat_statements/pg_stat_statements.c | 6 +- doc/src/sgml/xfunc.sgml | 4 +- src/backend/access/transam/slru.c | 4 +- src/backend/commands/async.c | 5 +- src/backend/postmaster/autovacuum.c | 6 +- src/backend/postmaster/bgwriter.c | 18 ++--- src/backend/replication/walreceiverfuncs.c | 20 ++--- src/backend/replication/walsender.c | 27 +++---- src/backend/storage/buffer/buf_table.c | 5 +- src/backend/storage/ipc/shmem.c | 81 ++++++++++--------- src/backend/storage/lmgr/lock.c | 6 +- src/backend/storage/lmgr/proc.c | 17 ++-- 12 files changed, 87 insertions(+), 112 deletions(-) diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 8fa249e9b8b..eb89aeca801 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -14,7 +14,7 @@ * Copyright (c) 2008-2010, PostgreSQL Global Development Group * * IDENTIFICATION - * $PostgreSQL: pgsql/contrib/pg_stat_statements/pg_stat_statements.c,v 1.13 2010/02/26 02:00:32 momjian Exp $ + * $PostgreSQL: pgsql/contrib/pg_stat_statements/pg_stat_statements.c,v 1.14 2010/04/28 16:54:15 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -321,8 +321,6 @@ pgss_shmem_startup(void) pgss = ShmemInitStruct("pg_stat_statements", sizeof(pgssSharedState), &found); - if (!pgss) - elog(ERROR, "out of shared memory"); if (!found) { @@ -343,8 +341,6 @@ pgss_shmem_startup(void) pgss_max, pgss_max, &info, HASH_ELEM | HASH_FUNCTION | HASH_COMPARE); - if (!pgss_hash) - elog(ERROR, "out of shared memory"); LWLockRelease(AddinShmemInitLock); diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index fcf9e3b1d49..1b771c5ecbf 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -1,4 +1,4 @@ - + User-Defined Functions @@ -3350,8 +3350,6 @@ void RequestAddinLWLocks(int n) LWLockAcquire(AddinShmemInitLock, LW_EXCLUSIVE); ptr = ShmemInitStruct("my struct name", size, &found); - if (!ptr) - elog(ERROR, "out of shared memory"); if (!found) { initialize contents of shmem area; diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c index 6226acc9283..846d1b8a949 100644 --- a/src/backend/access/transam/slru.c +++ b/src/backend/access/transam/slru.c @@ -41,7 +41,7 @@ * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/access/transam/slru.c,v 1.49 2010/02/16 22:34:43 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/slru.c,v 1.50 2010/04/28 16:54:15 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -164,8 +164,6 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns, shared = (SlruShared) ShmemInitStruct(name, SimpleLruShmemSize(nslots, nlsns), &found); - if (!shared) - elog(ERROR, "out of shared memory"); if (!IsUnderPostmaster) { diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index d3bb8d492f2..c3e783a8cae 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/async.c,v 1.156 2010/04/05 00:42:24 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/async.c,v 1.157 2010/04/28 16:54:15 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -464,9 +464,6 @@ AsyncShmemInit(void) asyncQueueControl = (AsyncQueueControl *) ShmemInitStruct("Async Queue Control", size, &found); - if (!asyncQueueControl) - elog(ERROR, "out of shared memory"); - if (!found) { /* First time through, so initialize it */ diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 8de6d878ddd..78f06672710 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -55,7 +55,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/postmaster/autovacuum.c,v 1.109 2010/02/26 02:00:55 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/postmaster/autovacuum.c,v 1.110 2010/04/28 16:54:15 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -2771,10 +2771,6 @@ AutoVacuumShmemInit(void) ShmemInitStruct("AutoVacuum Data", AutoVacuumShmemSize(), &found); - if (AutoVacuumShmem == NULL) - ereport(FATAL, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("not enough shared memory for autovacuum"))); if (!IsUnderPostmaster) { diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c index 9a3956a5094..72737ab2261 100644 --- a/src/backend/postmaster/bgwriter.c +++ b/src/backend/postmaster/bgwriter.c @@ -38,7 +38,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/postmaster/bgwriter.c,v 1.67 2010/02/05 23:37:43 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/postmaster/bgwriter.c,v 1.68 2010/04/28 16:54:15 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -890,16 +890,14 @@ BgWriterShmemInit(void) ShmemInitStruct("Background Writer Data", BgWriterShmemSize(), &found); - if (BgWriterShmem == NULL) - ereport(FATAL, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("not enough shared memory for background writer"))); - if (found) - return; /* already initialized */ - MemSet(BgWriterShmem, 0, sizeof(BgWriterShmemStruct)); - SpinLockInit(&BgWriterShmem->ckpt_lck); - BgWriterShmem->max_requests = NBuffers; + if (!found) + { + /* First time through, so initialize */ + MemSet(BgWriterShmem, 0, sizeof(BgWriterShmemStruct)); + SpinLockInit(&BgWriterShmem->ckpt_lck); + BgWriterShmem->max_requests = NBuffers; + } } /* diff --git a/src/backend/replication/walreceiverfuncs.c b/src/backend/replication/walreceiverfuncs.c index be305790fd3..78ee7fb9f7e 100644 --- a/src/backend/replication/walreceiverfuncs.c +++ b/src/backend/replication/walreceiverfuncs.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/replication/walreceiverfuncs.c,v 1.4 2010/02/26 02:00:57 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/replication/walreceiverfuncs.c,v 1.5 2010/04/28 16:54:15 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -58,17 +58,13 @@ WalRcvShmemInit(void) WalRcv = (WalRcvData *) ShmemInitStruct("Wal Receiver Ctl", WalRcvShmemSize(), &found); - if (WalRcv == NULL) - ereport(FATAL, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("not enough shared memory for walreceiver"))); - if (found) - return; /* already initialized */ - - /* Initialize the data structures */ - MemSet(WalRcv, 0, WalRcvShmemSize()); - WalRcv->walRcvState = WALRCV_STOPPED; - SpinLockInit(&WalRcv->mutex); + if (!found) + { + /* First time through, so initialize */ + MemSet(WalRcv, 0, WalRcvShmemSize()); + WalRcv->walRcvState = WALRCV_STOPPED; + SpinLockInit(&WalRcv->mutex); + } } /* Is walreceiver in progress (or starting up)? */ diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 6c41844b5f8..07f2b23c911 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -30,7 +30,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/replication/walsender.c,v 1.18 2010/04/28 16:10:42 heikki Exp $ + * $PostgreSQL: pgsql/src/backend/replication/walsender.c,v 1.19 2010/04/28 16:54:15 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -268,8 +268,7 @@ WalSndHandshake(void) if (wal_level == WAL_LEVEL_MINIMAL) ereport(FATAL, (errcode(ERRCODE_CANNOT_CONNECT_NOW), - errmsg("standby connections not allowed because wal_level='minimal'"))); - + errmsg("standby connections not allowed because wal_level=\"minimal\""))); /* Send a CopyOutResponse message, and start streaming */ pq_beginmessage(&buf, 'H'); @@ -838,21 +837,17 @@ WalSndShmemInit(void) WalSndCtl = (WalSndCtlData *) ShmemInitStruct("Wal Sender Ctl", WalSndShmemSize(), &found); - if (WalSndCtl == NULL) - ereport(FATAL, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("not enough shared memory for walsender"))); - if (found) - return; /* already initialized */ - - /* Initialize the data structures */ - MemSet(WalSndCtl, 0, WalSndShmemSize()); - - for (i = 0; i < max_wal_senders; i++) + if (!found) { - WalSnd *walsnd = &WalSndCtl->walsnds[i]; + /* First time through, so initialize */ + MemSet(WalSndCtl, 0, WalSndShmemSize()); - SpinLockInit(&walsnd->mutex); + for (i = 0; i < max_wal_senders; i++) + { + WalSnd *walsnd = &WalSndCtl->walsnds[i]; + + SpinLockInit(&walsnd->mutex); + } } } diff --git a/src/backend/storage/buffer/buf_table.c b/src/backend/storage/buffer/buf_table.c index 0751b8a0ffc..44b2e599bc2 100644 --- a/src/backend/storage/buffer/buf_table.c +++ b/src/backend/storage/buffer/buf_table.c @@ -15,7 +15,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/buffer/buf_table.c,v 1.51 2010/01/02 16:57:51 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/storage/buffer/buf_table.c,v 1.52 2010/04/28 16:54:15 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -66,9 +66,6 @@ InitBufTable(int size) size, size, &info, HASH_ELEM | HASH_FUNCTION | HASH_PARTITION); - - if (!SharedBufHash) - elog(FATAL, "could not initialize shared buffer hash table"); } /* diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c index 78106d01d58..5e6f9550227 100644 --- a/src/backend/storage/ipc/shmem.c +++ b/src/backend/storage/ipc/shmem.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/ipc/shmem.c,v 1.103 2010/01/02 16:57:51 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/storage/ipc/shmem.c,v 1.104 2010/04/28 16:54:16 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -213,13 +213,13 @@ InitShmemIndex(void) int hash_flags; /* + * Create the shared memory shmem index. + * * Since ShmemInitHash calls ShmemInitStruct, which expects the ShmemIndex * hashtable to exist already, we have a bit of a circularity problem in * initializing the ShmemIndex itself. The special "ShmemIndex" hash * table name will tell ShmemInitStruct to fake it. */ - - /* create the shared memory shmem index */ info.keysize = SHMEM_INDEX_KEYSIZE; info.entrysize = sizeof(ShmemIndexEnt); hash_flags = HASH_ELEM; @@ -227,8 +227,6 @@ InitShmemIndex(void) ShmemIndex = ShmemInitHash("ShmemIndex", SHMEM_INDEX_SIZE, SHMEM_INDEX_SIZE, &info, hash_flags); - if (!ShmemIndex) - elog(FATAL, "could not initialize Shmem Index"); } /* @@ -236,8 +234,9 @@ InitShmemIndex(void) * shared memory hash table. * * We assume caller is doing some kind of synchronization - * so that two people don't try to create/initialize the - * table at once. + * so that two processes don't try to create/initialize the same + * table at once. (In practice, all creations are done in the postmaster + * process; child processes should always be attaching to existing tables.) * * max_size is the estimated maximum number of hashtable entries. This is * not a hard limit, but the access efficiency will degrade if it is @@ -247,6 +246,10 @@ InitShmemIndex(void) * init_size is the number of hashtable entries to preallocate. For a table * whose maximum size is certain, this should be equal to max_size; that * ensures that no run-time out-of-shared-memory failures can occur. + * + * Note: before Postgres 9.0, this function returned NULL for some failure + * cases. Now, it always throws error instead, so callers need not check + * for NULL. */ HTAB * ShmemInitHash(const char *name, /* table string name for shmem index */ @@ -274,13 +277,6 @@ ShmemInitHash(const char *name, /* table string name for shmem index */ hash_get_shared_size(infoP, hash_flags), &found); - /* - * If fail, shmem index is corrupted. Let caller give the error message - * since it has more information - */ - if (location == NULL) - return NULL; - /* * if it already exists, attach to it rather than allocate and initialize * new space @@ -295,18 +291,20 @@ ShmemInitHash(const char *name, /* table string name for shmem index */ } /* - * ShmemInitStruct -- Create/attach to a structure in shared - * memory. + * ShmemInitStruct -- Create/attach to a structure in shared memory. * - * This is called during initialization to find or allocate + * This is called during initialization to find or allocate * a data structure in shared memory. If no other process * has created the structure, this routine allocates space * for it. If it exists already, a pointer to the existing - * table is returned. + * structure is returned. * - * Returns: real pointer to the object. FoundPtr is TRUE if - * the object is already in the shmem index (hence, already - * initialized). + * Returns: pointer to the object. *foundPtr is set TRUE if the object was + * already in the shmem index (hence, already initialized). + * + * Note: before Postgres 9.0, this function returned NULL for some failure + * cases. Now, it always throws error instead, so callers need not check + * for NULL. */ void * ShmemInitStruct(const char *name, Size size, bool *foundPtr) @@ -320,7 +318,9 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr) { PGShmemHeader *shmemseghdr = ShmemSegHdr; + /* Must be trying to create/attach to ShmemIndex itself */ Assert(strcmp(name, "ShmemIndex") == 0); + if (IsUnderPostmaster) { /* Must be initializing a (non-standalone) backend */ @@ -340,6 +340,12 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr) */ Assert(shmemseghdr->index == NULL); structPtr = ShmemAlloc(size); + if (structPtr == NULL) + ereport(ERROR, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("not enough shared memory for data structure" + " \"%s\" (%lu bytes requested)", + name, (unsigned long) size))); shmemseghdr->index = structPtr; *foundPtr = FALSE; } @@ -356,7 +362,8 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr) LWLockRelease(ShmemIndexLock); ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of shared memory"))); + errmsg("could not create ShmemIndex entry for data structure \"%s\"", + name))); } if (*foundPtr) @@ -364,15 +371,17 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr) /* * Structure is in the shmem index so someone else has allocated it * already. The size better be the same as the size we are trying to - * initialize to or there is a name conflict (or worse). + * initialize to, or there is a name conflict (or worse). */ if (result->size != size) { LWLockRelease(ShmemIndexLock); - - elog(WARNING, "ShmemIndex entry size is wrong"); - /* let caller print its message too */ - return NULL; + ereport(ERROR, + (errmsg("ShmemIndex entry size is wrong for data structure" + " \"%s\": expected %lu, actual %lu", + name, + (unsigned long) size, + (unsigned long) result->size))); } structPtr = result->location; } @@ -380,26 +389,24 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr) { /* It isn't in the table yet. allocate and initialize it */ structPtr = ShmemAlloc(size); - if (!structPtr) + if (structPtr == NULL) { - /* out of memory */ - Assert(ShmemIndex); + /* out of memory; remove the failed ShmemIndex entry */ hash_search(ShmemIndex, name, HASH_REMOVE, NULL); LWLockRelease(ShmemIndexLock); - - ereport(WARNING, + ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("could not allocate shared memory segment \"%s\"", - name))); - *foundPtr = FALSE; - return NULL; + errmsg("not enough shared memory for data structure" + " \"%s\" (%lu bytes requested)", + name, (unsigned long) size))); } result->size = size; result->location = structPtr; } - Assert(ShmemAddrIsValid(structPtr)); LWLockRelease(ShmemIndexLock); + + Assert(ShmemAddrIsValid(structPtr)); return structPtr; } diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index b4a2a6b0e0d..b196174f6e1 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/lmgr/lock.c,v 1.196 2010/03/21 00:17:58 petere Exp $ + * $PostgreSQL: pgsql/src/backend/storage/lmgr/lock.c,v 1.197 2010/04/28 16:54:16 tgl Exp $ * * NOTES * A lock table is a shared memory hash table. When @@ -306,8 +306,6 @@ InitLocks(void) max_table_size, &info, hash_flags); - if (!LockMethodLockHash) - elog(FATAL, "could not initialize lock hash table"); /* Assume an average of 2 holders per lock */ max_table_size *= 2; @@ -328,8 +326,6 @@ InitLocks(void) max_table_size, &info, hash_flags); - if (!LockMethodProcLockHash) - elog(FATAL, "could not initialize proclock hash table"); /* * Allocate non-shared hash table for LOCALLOCK structs. This stores lock diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 80775a4061f..97ba59a451d 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/lmgr/proc.c,v 1.217 2010/02/26 02:01:01 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/storage/lmgr/proc.c,v 1.218 2010/04/28 16:54:16 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -756,21 +756,22 @@ AuxiliaryProcKill(int code, Datum arg) /* * ProcQueueAlloc -- alloc/attach to a shared memory process queue * - * Returns: a pointer to the queue or NULL - * Side Effects: Initializes the queue if we allocated one + * Returns: a pointer to the queue + * Side Effects: Initializes the queue if it wasn't there before */ #ifdef NOT_USED PROC_QUEUE * -ProcQueueAlloc(char *name) +ProcQueueAlloc(const char *name) { + PROC_QUEUE *queue; bool found; - PROC_QUEUE *queue = (PROC_QUEUE *) - ShmemInitStruct(name, sizeof(PROC_QUEUE), &found); - if (!queue) - return NULL; + queue = (PROC_QUEUE *) + ShmemInitStruct(name, sizeof(PROC_QUEUE), &found); + if (!found) ProcQueueInit(queue); + return queue; } #endif