On 6 October 2014 11:22, Bernd <e...@zusammenkunft.net> wrote: > Dont worry, currentTimesMillis does not step backward if the timezone > changes automatically, as it is in UTC time. > > However it does jump if somebody manually corrects the time (or allows NTP > zu jump the time). Both operations are however a general problem on Java > servers, should be avoided. > > If you want to avoid the problem in your code, you can use the monotonic > nanoTime.
We started using nanoTime in JMeter and found that it could drift a significant amount from clock time. This is probably not an issue for POOL as the timeouts are not critical. [In JMeter we wanted accurate elapsed times even over long perriods] However, the ordering issue (if it exists) would still apply to the calculation window. Also the field must be volatile for safe publication. If it really is possible for statements in source code before a volatile access to be executed after the access, then we could just use a volatile variable for the current value as well. That might be cheaper than synch. But I am not sure that is required by the JVM. Seems to me that a lot of code would be affected by that and we would see many more problems. > Greetings > Bernd > Am 06.10.2014 09:35 schrieb "Jörg Schaible" <joerg.schai...@swisspost.com>: > >> Hi, >> >> Jacopo Cappellato wrote: >> >> > >> > On Oct 6, 2014, at 2:31 AM, sebb <seb...@gmail.com> wrote: >> > >> >>> The only way to do it is to make >> >>> lastReturnTime field thread-safe using locks. >> >> >> >> Volatile does make the field thread-safe. >> >> It's just a question of whether the JVM can re-order the statements >> >> when volatile is used. >> > >> > I think Xavier's comments are spot on. >> > From what I know, the only JMM guarantee that applies in this context is >> > the following: "A write to a volatile field happens-before every >> > subsequent read of that same field". >> > >> > On the other hand the "program order rule", as correctly pointed out by >> > Xavier, doesn't guarantee that the method will always return a positive >> > value because the JVM. >> > >> > I am aware of this issue in my fix. I have proposed this fix after I >> > learned that, by design (to avoid synchronization), in this class it was >> > relaxed the thread safe guarantees of a few (non critical) fields. >> > >> > However, the original issue I have identified and exposed in my unit test >> > could only happen under unlucky conditions (I don't know if they are >> > theoretical or real because I don't know enough about how it is used); >> the >> > "incomplete fix" that I have proposed makes the possibility of an error >> > even more remote (I was not able to recreate it with my unit test). >> > >> > In my opinion, if we do not want to implement a fix that involves thread >> > synchronization, we could enhance the fix by including the suggestion >> from >> > Bernd (returning null if the value is negative): >> > >> > Index: >> > src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java >> > =================================================================== >> > --- src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java >> > (revision 1629471) >> > +++ src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java >> > (working copy) @@ -85,7 +85,14 @@ >> > >> > @Override >> > public long getIdleTimeMillis() { >> > - return System.currentTimeMillis() - lastReturnTime; >> > + // Take a copy of lastReturnTime to prevent the risk that >> > + // a concurrent thread update the value of lastReturnTime >> > + // between the time that System.currentTimeMillis() >> > + // is called and before the return value is computed: in >> > + // this case a negative value could be returned >> > + long lrTime = lastReturnTime; >> > + long idleTimeMillis = System.currentTimeMillis() - lrTime; >> > + return idleTimeMillis > 0? idleTimeMillis: 0; >> > } >> > >> > @Override >> > >> > >> > What do you think? >> >> looking at this,I suppose the situation is even worse. >> System.currentTimeMillis will return the elapsed seconds since 1. January >> 1970 calculated based on the system time. Now, if the *system* time >> respects >> daylight saving, System.currentTimeMillis will also - so the value can jump >> back for an hour. System time is also affected by corrections from time >> daemons and might therefore decrease also. >> >> The current recommendation is to use System.nanoTime which calculates the >> elapsed time based on a value calculated at VM startup and that is not >> affected by changes in system time (well, it should not, but some JDK >> versions have errors here). Nevertheless, even in this situation you're not >> safe from negative values in the method above. If you run multi-threaded on >> several cores, the individual cores are not always synchronized by 100% >> (especially if they run at different clock-speeds due power management). >> Depending on the system timer used by the Java VM you are affected. >> >> So be always prepared to get a negative value for such time measuring >> methods. >> >> - Jörg >> >> >> --------------------------------------------------------------------- >> 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