Fix race in dsm_attach() when handles are reused.

DSM handle values can be reused as soon as the underlying shared memory
object has been destroyed.  That means that for a brief moment we
might have two DSM slots with the same handle.  While trying to attach,
if we encounter a slot with refcnt == 1, meaning that it is currently
being destroyed, we should continue our search in case the same handle
exists in another slot.

The race manifested as a rare "dsa_area could not attach to segment"
error, and was more likely in 10 and 11 due to the lack of distinct
seed for random() in parallel workers.  It was made very unlikely in
in master by commit 197e4af9, and older releases don't usually create
new DSM segments in background workers so it was also unlikely there.

This fixes the root cause of bug report #15585, in which the error
could also sometimes result in a self-deadlock in the error path.
It's not yet clear if further changes are needed to avoid that failure
mode.

Back-patch to 9.4, where dsm.c arrived.

Author: Thomas Munro
Reported-by: Justin Pryzby, Sergei Kornilov
Discussion: https://postgr.es/m/20190207014719[email protected]
Discussion: https://postgr.es/m/15585-324ff6a93a18da46@postgresql.org

diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index f1f75b73f50c056acb7b4354815b9d1a72f282cb..fde420d6edb539ac4c8a54d3af40575c23c8947b 100644 (file)
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -584,22 +584,20 @@ dsm_attach(dsm_handle h)
    nitems = dsm_control->nitems;
    for (i = 0; i < nitems; ++i)
    {
-       /* If the reference count is 0, the slot is actually unused. */
-       if (dsm_control->item[i].refcnt == 0)
+       /*
+        * If the reference count is 0, the slot is actually unused.  If the
+        * reference count is 1, the slot is still in use, but the segment is
+        * in the process of going away; even if the handle matches, another
+        * slot may already have started using the same handle value by
+        * coincidence so we have to keep searching.
+        */
+       if (dsm_control->item[i].refcnt <= 1)
            continue;
 
        /* If the handle doesn't match, it's not the slot we want. */
        if (dsm_control->item[i].handle != seg->handle)
            continue;
 
-       /*
-        * If the reference count is 1, the slot is still in use, but the
-        * segment is in the process of going away.  Treat that as if we
-        * didn't find a match.
-        */
-       if (dsm_control->item[i].refcnt == 1)
-           break;
-
        /* Otherwise we've found a match. */
        dsm_control->item[i].refcnt++;
        seg->control_slot = i;