>So (if my assumptions about method names being required are true) I>think I am >proposing: > >- fix pool >- release pool 2.4.4 >- update DBCP to pool 2.4.4 >- release DBCP
+1 sounds like a good plan to me. Closed pull request (either way, the other solution was to add back the previous behaviour, which means the tests would be fixed without my pull request anyway :) Cheers Bruno ________________________________ From: Mark Thomas <ma...@apache.org> To: Commons Developers List <dev@commons.apache.org> Sent: Tuesday, 31 October 2017 10:21 PM Subject: Re: [dbcp] update to pool 2.4.3. On 31/10/2017 00:54, Gary Gregory wrote: > On Sun, Oct 29, 2017 at 2:09 AM, Mark Thomas <ma...@apache.org> wrote: > >> On 29 October 2017 03:54:40 GMT+00:00, "Bruno P. Kinoshita" < >> brunodepau...@yahoo.com.br.INVALID> wrote: >>> Hi Gary, >>> >>> Started the tests in Maven command line, found which tests failed. >>> Executed the tests in Eclipse, found which class was related to the >>> failure. Then did a diff between both tags. >>> >>> git diff POOL_2_4_2 POOL_2.4.3-RC1 -- >>> ./src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java >>> >>> The short diff indicates that the failures started possibly due to the >>> replacement of Exceptions in the DefaultPooledObject by a CallStack. >>> Looks like the CallStack does not keep track of all the methods called >>> (due to a security context manager from what I could tell?). Only >>> classes. >>> >>> The following pull request updates pool to 2.4.3, and changes the unit >>> tests to, instead of looking for method names in the log writer output, >>> to look for the test class name. >>> >>> https://github.com/apache/commons-dbcp/pull/8 >>> >>> >>> Feel free to review and merge if you agree it's a good solution. >> >> I'm not sure it is a good solution. I need to look at what is going on in >> more detail but if the method information has been lost then that will make >> tracking down the root cause of a pool leak much, much harder. >> > > That we know that 2.4.3 does not provide method information compared to > 2.4.2, I propose: > > - Adapt the dbcp test to this new behavior, then release dbcp 2.2.0. > - Wait for Matt's fix to [poo], release [pool] 2.5.0 (and 2.4.4 if Matt > feels like providing a patch there as well.) > - Update dbcp again to test both the old and new call stack formats. > - Release dbcp 2.2.1 > > Thoughts? I'm not sure it makes sense to provide the class names without the methods. Are there use cases where that would be helpful? The only use case I'm aware of - leak detection - needs the methods to be present. It is a while since I looked this closely at Pool so I might be missing something. If the methods are required then that makes 2.4.3 broken in my view. In which case we should wait for 2.4.4 before updating the version DBCP depends on. I don't think we should adapt the test. The test is telling us something is broken. We should fix the root cause not change the test. So (if my assumptions about method names being required are true) I think I am proposing: - fix pool - release pool 2.4.4 - update DBCP to pool 2.4.4 - release DBCP 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
