On 2020-11-27 20:22:41 +0200, Heikki Linnakangas wrote: > On 26/11/2020 04:50, Tom Lane wrote: > > Craig Ringer <craig.ringer@enterprisedb.com> writes: > > > On Wed, Nov 25, 2020 at 9:23 PM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> > > > wrote: > > > > I'd prefer to make the lock self deadlock check run for production > > > > builds, not just cassert builds. > > > > I'd like to register a strong objection to spending any cycles whatsoever > > on this. If you're using LWLocks in a way that could deadlock, you're > > doing it wrong. The entire point of that mechanism is to be Light Weight > > and not have all the overhead that the heavyweight lock mechanism has. > > Building in deadlock checks and such is exactly the wrong direction. > > If you think you need that, you should be using a heavyweight lock. > > > > Maybe there's some case for a cassert-only check of this sort, but running > > it in production is right out. > > > > I'd also opine that checking for self-deadlock, but not any more general > > deadlock, seems pretty pointless. Careless coding is far more likely > > to create opportunities for the latter. (Thus, I have little use for > > the existing assertions of this sort, either.) > > I've made the mistake of forgetting to release an LWLock many times, leading > to self-deadlock. And I just reviewed a patch that did that this week [1]. > You usually find that mistake very quickly when you start testing though, I > don't think I've seen it happen in production.
I think something roughly along Craig's original patch, basically adding assert checks against holding a lock already, makes sense. Compared to the other costs of running an assert build this isn't a huge cost, and it's helpful.
I entirely concur that doing this outside of assertion builds is a seriously bad idea.
Yeah, given it only targets developer error that's sensible.
