On Sat, Aug 29, 2020 at 1:35 PM Phil Steitz <phil.ste...@gmail.com> wrote:
> A pool-related deadlock was reported recently in [1] to tomcat-user. > The OP was using a different pool, but it looks to me like the same > deadlock could happen with dbcp. The source is arguably a driver bug, > but in [2], the driver maintainer makes the good point that to avoid the > problem in [1] (and others like it) it would be better if pool > maintainers used abort instead of close to disconnect "stuck" or > abandoned connections. That makes sense to me. > > To implement this in dbcp, we would need to either always use abort > instead of close when closing physical connections (a bad idea, IMO, as > it could mess up counters due to async execution and add overhead) I agree that changing the happy path from close() to abort() is a bad idea; especially since the Connection#close() Javadoc contract releases database resources while abort() does not or is vague about it (at least in the Java 8 Javadoc). > or > modify pool to call a different factory method than destroy when > triggering close-abandoned. I think the latter makes sense and it could > be useful for other pool clients to be able to this kind of thing as well. > This means that we would treat abandoned connections like a real resource leak in the sense that (if) abort() leaks database resources, then the DBA would have to deal with that. I think formally surfacing an abort path vs. the current close/destroy is OK since it reflects what JDBC allows. If that means trickling the concept down to Pool, then that's OK as well IMO. This could be generically surfaced with a "close mode" enum passed to close/destroy APIs. This is similar to what we did in HttpCore's CloseMode(IMMEDIATE vs GRACEFUL): https://javadoc.io/static/org.apache.httpcomponents.core5/httpcore5/5.0.1/org/apache/hc/core5/io/CloseMode.html > What I propose is that we make these change to pool: > > 1. Add a new method to PooledObjectFactory called "abandonObject" with > default implementation delegating to destroyObject. > That or overload destroyObject with a destroyObject(CloseMode) would be easier to find. Gary > 2. Change the logic in removeAbandoned to call the new factory method > instead of destroyObject > > And then in dbcp: > > * Add implementation of abandonObject in PoolableConnectionFactory to > call abort. > > wdyt? > > Phil > > [1] > > https://lists.apache.org/thread.html/r7095d641bbc8db0a526d2d9a18684202347a747cf0e315a4a50d2001%40%3Cusers.tomcat.apache.org%3E > > [2] https://bugs.mysql.com/bug.php?id=64954 > > >
