Hello, I'm afraid the fix is wrong since, as Bernd said, no JMM property is used. The JVM is allowed to reorder statments and inlining statments. The volatile keyword assure you to have the most «fresh» value at the moment of the reading, but since the System.currentTimeMillis() has no side effect on the value of lastReturnTime you are not garantee of the non-inlining of the calculus. The only way to do it is to make lastReturnTime field thread-safe using locks. I'm new on this project, (and I confess I do not participate a lot to the apache community) and I just had a look at the class and I think I could found some time to fix some thread safty issues for this class.
Greetings 2014-10-05 14:28 GMT+02:00 sebb <seb...@gmail.com>: > On 5 October 2014 13:13, Bernd <e...@zusammenkunft.net> wrote: > > Hmm, > > > > I am not sure about this, the local variable fetch does not use any > > property of the Java Memory Model to make it gurantee to work. > > That's not the issue - see my recent update to the JIRA. > > > I would > > simply return 0 if the difference is negative. > > Not necessary. > > > And of course making the last used value volatile. > > +1 > > > Greetings > > Bernd > > > > BTW: for what is that idle time used? > > Am 05.10.2014 13:11 schrieb "Jacopo Cappellato (JIRA)" <j...@apache.org > >: > > > >> > >> [ > >> > https://issues.apache.org/jira/browse/POOL-279?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel > >> ] > >> > >> Jacopo Cappellato updated POOL-279: > >> ----------------------------------- > >> Attachment: POOL-279.patch > >> > >> New patch with a comment that explain the reason a copy of the field is > >> taken; thanks to [~s...@apache.org] for the review. > >> > >> > Thread concurrency issue in DefaultPooledObject.getIdleTimeMillis() > >> > ------------------------------------------------------------------- > >> > > >> > Key: POOL-279 > >> > URL: https://issues.apache.org/jira/browse/POOL-279 > >> > Project: Commons Pool > >> > Issue Type: Bug > >> > Affects Versions: 2.2 > >> > Reporter: Jacopo Cappellato > >> > Priority: Minor > >> > Attachments: POOL-279.patch, POOL-279.patch > >> > > >> > > >> > Under unlucky thread concurrency the getIdleTimeMillis() method of > >> DefaultPooledObject can return a negative value. > >> > I have attached a Junit test that fails most of the times and a simple > >> fix, that doesn't use synchronization: with this fix the Junit test > always > >> succeed. > >> > >> > >> > >> -- > >> This message was sent by Atlassian JIRA > >> (v6.3.4#6332) > >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > -- Xavier DETANT *"Vi veri veniversum vivus vici*" Johann Georg Faust
