On Wed, Apr 18, 2018 at 5:46 PM, Michael Paquier <mich...@paquier.xyz> wrote: > Personally, I favor approaches like that, because it allows to catch up > problems in using some APIs when people working on a patch miss any kind > of warning comments at the top of the function or within it which > summarize the conditions under which something needs to be used.
Right. Imagine how long it would take to figure out when there is a bug without something like this assertion. It's fairly difficult to debug LWLock deadlocks in production, even for experts. What I have in mind here is something that's a bit like AssertNotInCriticalSection(). We don't need to pepper AssertNotInCriticalSection() everywhere in practice, because calling palloc() is a pretty good proxy for "function should not be called in a critical section" -- palloc() calls AssertNotInCriticalSection(), which probably catches most unsafe code in critical sections immediately. We could probably also get decent Assert(!AnyBufferLockHeldByMe()) coverage without adding many new asserts. I'm curious about what we'll find by just by adding Assert(!AnyBufferLockHeldByMe()) to the top of heap_tuple_fetch_attr(). AssertNotInCriticalSection() certainly found several bugs when it was first added. -- Peter Geoghegan
