diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2017-06-05 12:05:42 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2017-06-05 12:05:42 -0400 |
commit | d4663350646ca0c069a36d906155a0f7e3372eb7 (patch) | |
tree | 0bc14a79fae8af098aeea21b7a018cf220c02a6e /src/backend/storage/ipc/shm_toc.c | |
parent | af51fea039bb8e00066d68d919312df1701dc03e (diff) |
Don't be so trusting that shm_toc_lookup() will always succeed.
Given the possibility of race conditions and so on, it seems entirely
unsafe to just assume that shm_toc_lookup() always finds the key it's
looking for --- but that was exactly what all but one call site were
doing. To fix, add a "bool noError" argument, similarly to what we
have in many other functions, and throw an error on an unexpected
lookup failure. Remove now-redundant Asserts that a rather random
subset of call sites had.
I doubt this will throw any light on buildfarm member lorikeet's
recent failures, because if an unnoticed lookup failure were involved,
you'd kind of expect a null-pointer-dereference crash rather than the
observed symptom. But you never know ... and this is better coding
practice even if it never catches anything.
Discussion: https://postgr.es/m/9697.1496675981@sss.pgh.pa.us
Diffstat (limited to 'src/backend/storage/ipc/shm_toc.c')
-rw-r--r-- | src/backend/storage/ipc/shm_toc.c | 10 |
1 files changed, 9 insertions, 1 deletions
diff --git a/src/backend/storage/ipc/shm_toc.c b/src/backend/storage/ipc/shm_toc.c index 9110ffa4a0b..5e290df3366 100644 --- a/src/backend/storage/ipc/shm_toc.c +++ b/src/backend/storage/ipc/shm_toc.c @@ -208,6 +208,9 @@ shm_toc_insert(shm_toc *toc, uint64 key, void *address) /* * Look up a TOC entry. * + * If the key is not found, returns NULL if noError is true, otherwise + * throws elog(ERROR). + * * Unlike the other functions in this file, this operation acquires no lock; * it uses only barriers. It probably wouldn't hurt concurrency very much even * if it did get a lock, but since it's reasonably likely that a group of @@ -215,7 +218,7 @@ shm_toc_insert(shm_toc *toc, uint64 key, void *address) * right around the same time, there seems to be some value in avoiding it. */ void * -shm_toc_lookup(shm_toc *toc, uint64 key) +shm_toc_lookup(shm_toc *toc, uint64 key, bool noError) { uint64 nentry; uint64 i; @@ -226,10 +229,15 @@ shm_toc_lookup(shm_toc *toc, uint64 key) /* Now search for a matching entry. */ for (i = 0; i < nentry; ++i) + { if (toc->toc_entry[i].key == key) return ((char *) toc) + toc->toc_entry[i].offset; + } /* No matching entry was found. */ + if (!noError) + elog(ERROR, "could not find key " UINT64_FORMAT " in shm TOC at %p", + key, toc); return NULL; } |