FWIW, I like the name "DestroyMode" because it matches the "destroy" in the method name.
Gary On Mon, Sep 7, 2020, 19:08 Gary Gregory <garydgreg...@gmail.com> wrote: > On Mon, Sep 7, 2020 at 6:02 PM Phil Steitz <phil.ste...@gmail.com> wrote: > > > > > > On 9/3/20 2:44 AM, Mark Thomas wrote: > > > On 31/08/2020 01:05, Phil Steitz wrote: > > > > > > <snip/> > > > > > >> If others agree it is a good idea for dbcp, I can do it. I can see > the > > >> argument that its better to stay with close() even for abandoned and I > > >> have not been able to get the deadlock to happen, so I would like to > > >> wait a bit and allow others to weigh in. Similarly for pool, I would > > >> like to get more community feedback before adding to the interface. > > > Hmm. > > > > > > On one hand if the driver deadlocks I don't see how that can be > anything > > > other than a driver bug. If multiple code paths obtain multiple locks > > > then those code paths must always obtain those locks in the same order. > > > Anything else is a bug that is likely to result in a deadlock in a > > > multithreaded environment. > > > > > > On the other hand, it could be argued that the situation only arises > > > when an application doesn't correctly return connections to the pool > > > and/or keeps them for too long and/or doesn't configure the pool > > > correctly for their usage pattern. > > > > > > The approach of adding > > > > > > PooledObjectFactory.destroyObject(PooledObject,CloseMode) > > > > > > where CloseMode is an Enum with two values looks reasonable to me. > > > > > > I have started to work on the [pool] changes for this. I want to check > > two things before completing the PR: > > > > 1. "Close" is a [dbcp] concept which does not make sense for all pool > > factories, so I am going to name the enum "DestroyMode" and the two > > modes, "Default" and "Abandoned". That leaves room for other modes like > > "Evicted" or "Invalid" later. > > > > 2. Speaking of later, technically adding modes will not break binary > > compatibility. Are we going to be OK adding outside a major release? > > If the answer is no, I might argue to include the other natural modes > now. > > Yes, IMO, it is OK to add enum values in a minor release since it does > not break binary (or source) compatibility. > > Gary > > > > > Phil > > > > > > > > I do agree that abort() should only be used in the case of abandoned > > > connections. > > > > > > Mark > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > >
