<Swing Dev> [9] Review Request for 8130735: javax.swing.TimerQueue: timer fires late when another timer starts
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Tue Jul 21 07:35:59 UTC 2015
What about to add removed flat to DelayedTimer? Something like:
TimerQueue.removeTimer(Timer timer){
timer.lock();
// ...
timer.delayedTimer.removed = true;
queue.remove(timer.delayedTimer);
timer.delayedTimer = null;
timer.unlock();
}
TimerQueue.run(){
DelayedTimer delayedTimer = queue.take();
delayedTimer.getTimer().lock();
if(delayedTimer.removed){
// skip it
}
// ...
delayedTimer.getTimer().unlock();
}
Thanks,
Alexandr.
On 7/17/2015 12:28 AM, Sergey Bylokhov wrote:
> 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.
More information about the swing-dev
mailing list