Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Jan 6, 2014 at 3:32 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I agree it'd be nicer if we had some better way than mere manual
>> inspection to enforce proper use of spinlocks; but this change doesn't
>> seem to me to move the ball downfield by any meaningful distance.
> Well, my thought was mainly that, while it may be a bad idea to take
> another spinlock while holding a spinlock under any circumstances,
> somebody might do it and it might appear to work just fine. The most
> likely sequences seems to me to be something like SpinLockAcquire(...)
> followed by LWLockConditionalAcquire(), thinking that things are OK
> because the lock acquisition is conditional - but in fact the
> conditional acquire still takes the spinlock unconditionally.
The point I'm making is that no such code should get past review,
whether it's got an obvious performance problem or not.
There's a huge amount of stuff that in principle we could add overhead
to Assert-enabled builds for, but we prefer not to --- an example not
too far afield from this issue is that there's no mechanism for detecting
deadlocks among multiple LWLock holders. If we go too far down the path
of adding useless checks to Assert builds, people will stop using such
builds for routine development, which would surely be a large net negative
reliability-wise.
I'd be okay with adding overhead to SpinLockAcquire/Release if it had some
meaningful probability of catching real bugs, but I don't see that that
claim can be made here.
regards, tom lane