Re: Is element access after HASH_REMOVE ever OK?

Hi,

On 2021-05-10 20:15:41 -0400, Tom Lane wrote:
> I wrote:
> > ... Can we get rid of the unsafe
> > access easily?
>
> Oh, shoulda read your second patch first. Looking at that,
> I fear it might not be quite that simple, because the
> comment on CheckAndSetLockHeld says very clearly
>
> * It is callers responsibility that this function is called after
> * acquiring/releasing the relation extension/page lock.
>
> so your proposed patch violates that specification.

It wouldn't be too hard to fix this though - we can just copy the
locktag into a local variable. Or use one of the existing local copies,
higher in the stack.

But:

> I'm inclined to think that this API spec is very poorly thought out
> and should be changed --- why is it that the flags should change
> *after* the lock change in both directions? But we'd have to take
> a look at the usage of these flags to understand what's going on
> exactly.

I can't see a need to do it after the HASH_REMOVE at least - as we don't
return early if that fails, there's no danger getting out of sync if we
reverse the order. I think the comment could just be changed to say
that the function has to be called after it is inevitable that the lock
is acquired/released.

Greetings,

Andres Freund