<AWT Dev> [8] Review request for 2228674: Fix failed for CR 7162144

[Oleg Pekhovskiy]

Hi Anthony,

thank you for the review.

As for your question I could just refer to your fix for JDK 7 where the 
following changes we made in
src/share/classes/java/awt/EventDispatchThread.java:

@@ -100,7 +94,12 @@ class EventDispatchThread extends Thread
                      }
                  });
              } finally {
-                if(getEventQueue().detachDispatchThread(this, shutdown)) {
+                // 7189350: doDispatch is reset from stopDispatching(),
+                //    on InterruptedException, or ThreadDeath. Either way,
+                //    this indicates that we must force shutting down.
+                if (getEventQueue().detachDispatchThread(this,
+                            !doDispatch || isInterrupted()))
+                {
                      break;
                  }
              }

Condition "!doDispatch || isInterrupted()" is always true and as a 
result forced detach is always performed. That's why in 
EventQueue.detachDispatchThread() method in condition
"!forceDetach && (peekEvent() != null)" we never reach "peekEvent() != 
null" and therefore we could simply remove it.

Thanks,
Oleg

On 16.10.2013 15:29, Anthony Petrov wrote:
> Hi Oleg,
>
> The fix looks good to me. However, I don't recall all details about this
> machinery, and removing a call to peekEvent() in
> EventQueue.detachDispatchThread() looks a bit scary.
>
> Could Artem or you clarify if AWTAutoShutdown() recreates the EDT if
> there are still events in the queue, or we simply don't care about them?
>
> --
> best regards,
> Anthony
>
> On 10/16/2013 02:31 PM, Oleg Pekhovskiy wrote:
>> Hi all,
>>
>> please review the fix
>> http://cr.openjdk.java.net/~bagiras/2228674.1/
>> for
>> https://bugs.openjdk.java.net/browse/JDK-2228674
>>
>> The fix covers two scenarios:
>> 1. User code calls EventDispatchThread.interrupt() and then
>> EventDispatchThread.interrupted() which clears interrupted state and
>> EDT doesn't stop.
>>
>> 2. EventDispatchThread.interrupt() is called without clearing the
>> interrupted state (e.g. invocation of AppContext.dispose()) that makes
>> EDT terminate.
>>
>> The other scenario, in which AppContext.dispose() is called from the
>> thread other than EDT and after that EventDispatchThread.interrupted()
>> is called from EDT preventing EDT from termination, is treated like an
>> architecture bug.
>>
>> Some dead code was also removed because detaching of EDT is always
>> forced.
>>
>> Thanks,
>> Oleg