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
