Re: Portability issues in shm_mq

On Sun, Mar 16, 2014 at 11:26 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Fri, Mar 14, 2014 at 4:43 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> How is that leading to a crash? Well, this machine is 32-bit, so MAXALIGN
>>> is only 4. This means it is possible for an odd-length message cum
>>> message length word to not exactly divide the size of the shared memory
>>> ring buffer, resulting in cases where an 8-byte message length word is
>>> wrapped around the end of the buffer.
>
>> Argh. I think I forced the size of the buffer to be MAXALIGN'd, but
>> what it really needs is to be a multiple of the size of uint64.
>
> After sleeping on it, I think what you're proposing here is to double down
> on a wrong design decision. ISTM you should change the message length
> words to be size_t (or possibly ssize_t, if you're depending on signed
> arithmetic), which would let you keep using MAXALIGN as the alignment
> macro. There is absolutely no benefit, either for performance or code
> readability, in forcing 32-bit machines to use 64-bit message length
> words. Indeed, by not using the same alignment macros as everywhere else
> and not being able to use %zu for debug printouts, I think the only real
> effect you're producing is to make the DSM/MQ stuff more and more randomly
> unlike the rest of Postgres. Please reconsider while it's still not too
> late to change those APIs.

Hmm. That's not a terrible idea. I think part of the reason I did it
this way because, although the size of an individual message can be
limited to size_t, the queue maintains a counter of the total number
of bytes ever sent and received, and that has to use 64-bit arithmetic
so it doesn't overflow (much as we do for LSNs). It seemed simpler to
make all of the lengths uint64 rather than the lengths of individual
messages Size and the total number of bytes sent and received uint64.
However, that could probably be worked out.

But I think there's another possible problem here. In order for reads
from the buffer not to suffer alignment problems, the chunk size for
reads and writes from the buffer needs to be MAXIMUM_ALIGNOF (or some
multiple of it). And in order to avoid a great deal of additional and
unwarranted complexity, the size of the message word also needs to be
MAXIMUM_ALIGNOF (or some multiple of it). So the message word can
only be of size 4 if MAXIMUM_ALIGNOF is also 4. IOW, I think your
approach is going to run into trouble on any system where
sizeof(Size)==4 but MAXIMUM_ALIGNOF==8.

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