Hi, Semyon.
I see that the chance to reproduce the problem is very very small,
because we should call addTimer, when we are at lines 171/172. So the
bug is about really small timings. So the related question: Is it
possible in the fixed version to call addTimer when we remove
DelayedTimer from the queue via queue.take(), but before we assign its
value to the runningTimer?
On 14.07.15 12:51, Alexander Zvegintsev wrote:
still looks good to me.
Thanks,
Alexander.
On 07/14/2015 12:41 PM, Semyon Sadetsky wrote:
Hi Alexander,
I added the double check
:http://cr.openjdk.java.net/~ssadetsky/8130735/webrev.01/
--Semyon
On 7/13/2015 1:24 PM, Alexander Zvegintsev wrote:
Hello Semyon,
the fix looks good to me.
P.S. Just a side note, as I can see we could possibly start two
threads instead of one in startIfNeeded():
96 void startIfNeeded() {
97 if (! running) {
98 runningLock.lock();
99 try {
100 final ThreadGroup threadGroup =
AppContext.getAppContext().getThreadGroup();
101 AccessController.doPrivileged((PrivilegedAction<Object>) () -> {
102 String name = "TimerQueue";
103 Thread timerThread = new
ManagedLocalsThread(threadGroup,
104 this, name);
!running check is missing after try. It is not the case with current
code base, but it may be changed in future.
Thanks,
Alexander.
On 07/09/2015 08:08 PM, Semyon Sadetsky wrote:
Hello,
Please review fix for JDK9:
bug: https://bugs.openjdk.java.net/browse/JDK-8130735
webrev: http://cr.openjdk.java.net/~ssadetsky/8130735/webrev.00/
The root cause is the setting larger expiration time for the timer
which is already inserted into the delay queue. So all timers
behind the timer cannot be executed earlier than its expiration
time. This happens very rare only for repeated timers and only if
user uses the Swing timer API inaccurately (call start() without
stop()).
The fix eliminates this possibility by introducing a check if the
timer was already restarted concurrently.
It is difficult to write test because I could not reliably
reproduce the issue for a reasonable time.
--Semyon
--
Best regards, Sergey.
