On Wed, Apr 18, 2018 at 5:46 PM, Michael Paquier <michael@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