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 > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org