Hi,
On 2022-11-17 14:20:47 -0500, Robert Haas wrote:
> On Wed, Nov 16, 2022 at 8:42 PM Andres Freund <andres@anarazel.de> wrote:
> > Afaict the problem is that
> > proc = (PGPROC *) &(waitQueue->links);
> >
> > is a gross gross hack - this isn't actually a PGPROC, it's pointing to an
> > SHM_QUEUE, but *not* one embedded in PGPROC. It kinda works because ->links
> > is at offset 0 in PGPROC, which means that
> > SHMQueueInsertBefore(&(proc->links), &(MyProc->links));
> > will turn &proc->links back into waitQueue->links. Which we then can enqueue
> > again.
>
> Not that I object to a targeted fix
Should we backpatch this fix? Likely this doesn't cause active breakage
outside of 32bit builds under ubsan, but that's not an unreasonable thing to
want to do in the backbranches.
> but it's been 10 years since
> slist and dlist were committed, and we really ought to eliminate
> SHM_QUEUE entirely in favor of using those. It's basically an
> open-coded implementation of something for which we now have a
> toolkit. Not that it's impossible to make this kind of mistake with a
> toolkit, but in general open-coding the same logic in multiple places
> increases the risk of bugs.
Agreed. I had started on a set of patches for some of the SHM_QUEUE uses, but
somehow we ended up a bit stuck on the naming of dlist_delete variant that
afterwards zeroes next/prev so we can replace SHMQueueIsDetached() uses.
Should probably find and rebase those patches...
Greetings,
Andres Freund
