Robert Haas <robertmh...@gmail.com> writes: > On Mon, Jun 5, 2017 at 11:19 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: >> I think that shm_toc_lookup() ought to be made to throw elog(ERROR) on an >> unexpected failure. To satisfy the one caller that doesn't want an error, >> we could either add a "bool noError" parameter to it, or split it into two >> functions shm_toc_lookup() and shm_toc_lookup_noerror(). The latter would >> require touching less code, but the former is probably more like what we'd >> have had if this were designed more carefully to begin with.
> Either is OK with me. Done with the extra parameter. If we discover a reason to back-patch this, we'd likely need to do it the other way in back branches, but for now I'll refrain. While I'm looking at this ... seems like there's a pretty basic coding- rule violation here, namely that shm_toc_lookup() thinks it can read toc->toc_nentry without any sort of locking. Since that field is declared Size, this amounts to an assumption that 64-bit reads are atomic, which is not per project practice. In practice it probably can't fail even if 64-bit reads aren't atomic, simply because we'll never have enough entries in a shm_toc to make the high-order half ever change. But that just begs the question why the field is declared Size rather than int. I think we should make it the latter. I am also thinking that most of the shm_toc functions need to have the toc pointers declared as "volatile *", but particularly shm_toc_lookup. That read_barrier call might prevent the hardware from reordering accesses, but I don't think it stops the compiler from doing so. These problems are probably just latent, because it looks to me like in our current usage no process would ever be doing lookups on shm_tocs that are being changed concurrently. But they'll bite us someday. regards, tom lane -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers