When you catch InterruptedException, you must do one of 3 things:
1 - handle it
2 - rethrow it
3 - re-assert thread interrupted status
(Thread.currentThread().interrupt()) so that it will be thrown at some
other (more appropriate) place/time.
So if you don't want to complicate code and API to do 2, you can do 3.
Regards, Peter
On Aug 31, 2012 4:50 PM, "Anthony Petrov" <anthony.pet...@oracle.com> wrote:
> I think you should re-throw the InterruptedException caught at flush().
> BTW, the flush() method can be invoked on a thread other than EDT (at least
> in theory).
>
> --
> best regards,
> Anthony
>
> On 8/30/2012 9:01 PM, Oleg Pekhovskiy wrote:
>
>> Hi,
>>
>> I got another idea preparing the next version of fix.
>>
>> Previously we didn't catch InterruptedException and stack unwinding took
>> place right up to
>> try-catch section in EventDispatchThread.**pumpOneEventForFilters().
>>
>> So seems like it would be correct not eating that exception in
>> PostEventQueue.flush()
>> but just check the state using isInterrupted() method and add 'throws
>> InterruptedException'
>> to PostEventQueue.flush() and SunToolkit.flushPendingEvents(**) methods.
>>
>> Thoughts?
>>
>> Thanks,
>> Oleg
>>
>> 8/30/2012 5:20 PM, Anthony Petrov wrote:
>>
>>> I see. After giving it more thought I don't see an easy solution for
>>> this issue, too. As long as we guarantee that the EDT is recreated should
>>> more events be posted, I don't see a big problem with this. So let's stick
>>> with the "Minimize..." approach then.
>>>
>>> --
>>> best regards,
>>> Anthony
>>>
>>> On 08/30/12 00:18, Oleg Pekhovskiy wrote:
>>>
>>>> Hi Anthony,
>>>>
>>>> I see your concerns.
>>>>
>>>> As PostEventQueue.flush() method left unsynchronized,
>>>> we potentially could return PostEventQueue.noEvents()
>>>> and return check in EventQueue.**detachDispatchThread()
>>>> back to the condition.
>>>> But is could increase the possibility of deadlock in future
>>>> (with PostEventQueue & pushPopLock).
>>>>
>>>> Artem, what do you think?
>>>>
>>>> Thanks,
>>>> Oleg
>>>>
>>>> 29.08.2012 15:22, Anthony Petrov wrote:
>>>>
>>>>> On 8/29/2012 3:08 PM, Anthony Petrov wrote:
>>>>>
>>>>>> Hi Oleg,
>>>>>>
>>>>>> I'm still concerned about the following:
>>>>>>
>>>>>> detachDispatchThread()
>>>>>> {
>>>>>> flush();
>>>>>> lock();
>>>>>> // possibly detach
>>>>>> unlock();
>>>>>> }
>>>>>>
>>>>>> at EventQueue.java. What if an even get posted to the queue after the
>>>>>>
>>>>>
>>>>> A typo: s/even get/event gets/.
>>>>>
>>>>> --
>>>>> best regards,
>>>>> Anthony
>>>>>
>>>>> flush() returns but before we even acquired the lock? We may still
>>>>>> end up with a situation when we detach the thread while in fact there
>>>>>> are some pending events present, which actually contradicts the
>>>>>> current logic of the detach() method. I see that you say "Minimize
>>>>>> discard possibility" in the comment instead of "Prevent ...", but I
>>>>>> feel uncomfortable with this actually.
>>>>>>
>>>>>> What exactly prevents us from adding some synchronization to ensure
>>>>>> that the detaching only happens when there's really no pending events?
>>>>>>
>>>>>> SunToolkit.java:
>>>>>>
>>>>>>> 2120 Boolean b = isThreadLocalFlushing.get();
>>>>>>> 2121 if (b != null && b) {
>>>>>>> 2122 return;
>>>>>>> 2123 }
>>>>>>> 2124 2125 isThreadLocalFlushing.set(**true);
>>>>>>> 2126 try {
>>>>>>>
>>>>>>
>>>>>> How does access to the isThreadLocalFlushing synchronized? What
>>>>>> happens if the flush() is being invoked from two different threads
>>>>>> for the same post event queue? Why do we have two "isFlushing" flags?
>>>>>> Can we collapse them into one? Why is the isFlushing set/reset in two
>>>>>> disjunct synchronized(){} blocks?
>>>>>>
>>>>>> Overall, I find the current synchronization scheme in flush() very,
>>>>>> *very* (and I mean it) confusing. Can we simplify it somehow?
>>>>>>
>>>>>> --
>>>>>> best regards,
>>>>>> Anthony
>>>>>>
>>>>>> On 8/28/2012 6:33 PM, Oleg Pekhovskiy wrote:
>>>>>>
>>>>>>> Hi Artem, Anthony,
>>>>>>>
>>>>>>> thank you for your proposals!
>>>>>>>
>>>>>>> We with Artem also had off-line discussion,
>>>>>>> so as a result I prepared improved version of fix:
>>>>>>> http://cr.openjdk.java.net/~**bagiras/8/7186109.3/<http://cr.openjdk.java.net/~bagiras/8/7186109.3/>
>>>>>>>
>>>>>>> What was done:
>>>>>>> 1. EventQueue.**detachDispatchThread(): moved
>>>>>>> SunToolkit.flushPnedingEvents(**) above the comments and added a
>>>>>>> separate comment to it.
>>>>>>> 2. Moved SunToolkitSubclass.**flushPendingEvents(AppContext) method
>>>>>>> to
>>>>>>> SunToolkit. Deleted SunToolkitSubclass.
>>>>>>> 3. Moved isFlushingPendingEvents to PostEventQueue with the new name
>>>>>>> - isThreadLocalFlushing and made it ThreadLocal.
>>>>>>> 4. Left PostEventQueue.flush() unsynchronized and created
>>>>>>> wait()-notifyAll() synchronization mechanism to avoid blocking of
>>>>>>> PostEventQueue.postEvent().
>>>>>>>
>>>>>>> Looking forward to your comments!
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Oleg
>>>>>>>
>>>>>>> 20.08.2012 20:20, Artem Ananiev wrote:
>>>>>>>
>>>>>>>> Hi, Oleg,
>>>>>>>>
>>>>>>>> here are a few comments:
>>>>>>>>
>>>>>>>> 1. What is the reason of keeping "isFlushingPendingEvents" in
>>>>>>>> SunToolkit, given that PEQ.flush() is synchronized (and therefore
>>>>>>>> serialized) anyway?
>>>>>>>>
>>>>>>>> 2. flushPendingEvents(AppContext) may be moved directly to
>>>>>>>> SunToolkit, so we don't need a separate sun-class for that.
>>>>>>>>
>>>>>>>> 3. EQ.java:1035-1040 - this comment is obsolete and must be
>>>>>>>> replaced by another one.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Artem
>>>>>>>>
>>>>>>>> On 8/17/2012 4:49 PM, Oleg Pekhovskiy wrote:
>>>>>>>>
>>>>>>>>> Hi!
>>>>>>>>>
>>>>>>>>> Please review the fix for CR:
>>>>>>>>> http://bugs.sun.com/**bugdatabase/view_bug.do?bug_**id=7186109<http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7186109>
>>>>>>>>>
>>>>>>>>> Webrev:
>>>>>>>>> http://cr.openjdk.java.net/~**bagiras/8/7186109.1/<http://cr.openjdk.java.net/~bagiras/8/7186109.1/>
>>>>>>>>>
>>>>>>>>> The following changes were made:
>>>>>>>>> 1. Removed flushLock from SunToolkit.flushPendingEvent()
>>>>>>>>> 2. Returned method PostEventQueue.flush() as 'synchronized' back
>>>>>>>>> 3. Added call of SunToolkit.flushPendingEvents(**) to
>>>>>>>>> EventQueue.**detachDispatchThread(),
>>>>>>>>> right before pushPopLock.lock()
>>>>>>>>> 4. Removed !SunToolkit.**isPostEventQueueEmpty() check from
>>>>>>>>> EventQueue.**detachDispatchThread()
>>>>>>>>> 5. Removed SunToolkit.**isPostEventQueueEmpty() &
>>>>>>>>> PostEventQueue.noEvents();
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Oleg
>>>>>>>>> <http://cr.openjdk.java.net/%**7Ebagiras/8/7186109.1/<http://cr.openjdk.java.net/%7Ebagiras/8/7186109.1/>
>>>>>>>>> >
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>
>>
