Re: possible dsm bug in dsm_attach()

On Tue, May 6, 2014 at 8:43 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> dsm_attach() does the following:
>
>         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)
>                         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, if the descriptor matches, we've found a match. */
>                 if (dsm_control->item[i].handle == seg->handle)
>                 {
>                         dsm_control->item[i].refcnt++;
>                         seg->control_slot = i;
>                         break;
>                 }
>         }
>
> The break because of refcnt == 1 doesn't generally seem to be a good
> idea. Why are we bailing if there's *any* segment that's in the process
> of being removed? I think the check should be there *after* the
> dsm_control->item[i].handle == seg->handle check?

You are correct.  Good catch.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company