On Wed, 30 Dec 2020 at 10:11, Andres Freund <and...@anarazel.de> wrote:
> Hi, > > On 2020-11-27 20:22:41 +0200, Heikki Linnakangas wrote: > > On 26/11/2020 04:50, Tom Lane wrote: > > > Craig Ringer <craig.rin...@enterprisedb.com> writes: > > > > On Wed, Nov 25, 2020 at 9:23 PM Ashutosh Bapat < > ashutosh.bapat....@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.
