<AWT Dev> <Awt Dev> [9] Review Request for 8130895: Test javax/swing/system/6799345/TestShutdown.java fails on Solaris11 Sparcv9

[Alexander Scherbatiy]

   The fix looks good to me.

   Thanks,
   Alexandr.

On 10/21/2015 5:55 PM, Sergey Bylokhov wrote:
> Thanks for clarification, the fix looks fine then.
>
> On 21.10.15 10:16, Semyon Sadetsky wrote:
>>
>>
>> On 10/20/2015 6:13 PM, Sergey Bylokhov wrote:
>>> On 20.10.15 13:33, Semyon Sadetsky wrote:
>>>>
>>>>
>>>> On 10/19/2015 8:12 PM, Sergey Bylokhov wrote:
>>>>> On 18.09.15 18:50, Semyon Sadetsky wrote:
>>>>>>> Why do you think so? Each app context KFM has own keystrokes if 
>>>>>>> it is
>>>>>>> initialized from the thread that belongs to its ThreadGroup.
>>>>>> Sorry. I see what you mean. That is corrected as well:
>>>>>> http://cr.openjdk.java.net/~ssadetsky/8130895/webrev.02/
>>>>>
>>>>> After the changes in KeyboardFocusManager.java it seems it is
>>>>> unnecessary to add the KfmAccessor holder in
>>>>> KeyboardFocusManagerPeerImpl?
>>>> Why? KfmAccessor just fixes the bug because KFM should be created 
>>>> on the
>>>> corresponding EDT thread bounded with the application context.
>>>
>>> But the old code in KeyboardFocusManagerPeerImpl did not create the
>>> new KFM, it saves an accessor to the class of KFM, which causes an
>>> initialization of KFM class and uses of Appcontext data in static
>>> initialization.
>>> But the problem with static initialization was fixed in
>>> KeyboardFocusManager.java, why we still need the holder?
>>>
>> Right. It also fixes new KFM instance. But KFM class is not needed for
>> peer.getCurrentFocusOwner(). If, for example, a AWT robot is used
>> without creation any components, the KFM class is not used. Its static
>> initialization is rather heavy and consumes RAM. So I would keep KFM
>> initialization lazy for that reason.
>>>
>>> It works
>>>> because XAWT and system EDT share the same thread group but 
>>>> generally it
>>>> is not valid to request app context on XAWT thread. And KFM 
>>>> instance is
>>>> not used by XAWT it only needs the peer.
>>>>>
>>>>>
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --Semyon
>>>>>>>>>>>
>>>>>>>>>>> On 7/23/2015 7:38 AM, Semyon Sadetsky wrote:
>>>>>>>>>>>> Hi Sergey,
>>>>>>>>>>>>
>>>>>>>>>>>> As you said there are 2 issues here. The first is about
>>>>>>>>>>>> KeyboardFocusManager initialization in the main event loop.
>>>>>>>>>>>> And the second about initialization of the
>>>>>>>>>>>> KeyboardManagerManager
>>>>>>>>>>>> itself where keystrokes are shared between contexts.
>>>>>>>>>>>> Or do you mean if I remove the keystrokes sharing then the
>>>>>>>>>>>> KeyboardFocusManager can be initialized in the toolkit
>>>>>>>>>>>> thread? Is
>>>>>>>>>>>> that what you mean?
>>>>>>>>>>>>
>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>
>>>>>>>>>>>> On 7/22/2015 8:20 PM, Sergey Bylokhov wrote:
>>>>>>>>>>>>> It is unclear why it is unrelated, the stack trace from the
>>>>>>>>>>>>> bug:
>>>>>>>>>>>>>
>>>>>>>>>>>>> java.lang.ExceptionInInitializerError
>>>>>>>>>>>>> at sun.misc.Unsafe.ensureClassInitialized(Native Method)
>>>>>>>>>>>>> at
>>>>>>>>>>>>> sun.awt.AWTAccessor.getKeyboardFocusManagerAccessor(AWTAccessor.java:966) 
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> at
>>>>>>>>>>>>> sun.awt.KeyboardFocusManagerPeerImpl.<clinit>(KeyboardFocusManagerPeerImpl.java:46) 
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> at sun.awt.X11.XToolkit.run(XToolkit.java:611)
>>>>>>>>>>>>> at sun.awt.X11.XToolkit.run(XToolkit.java:550)
>>>>>>>>>>>>> at java.lang.Thread.run(Thread.java:745)
>>>>>>>>>>>>> Caused by: java.lang.NullPointerException
>>>>>>>>>>>>> at 
>>>>>>>>>>>>> java.awt.AWTKeyStroke.getCachedStroke(AWTKeyStroke.java:255)
>>>>>>>>>>>>> at 
>>>>>>>>>>>>> java.awt.AWTKeyStroke.getAWTKeyStroke(AWTKeyStroke.java:394)
>>>>>>>>>>>>> at
>>>>>>>>>>>>> java.awt.KeyboardFocusManager.<clinit>(KeyboardFocusManager.java:332) 
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> ... 6 more
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 22.07.15 17:09, Semyon Sadetsky wrote:
>>>>>>>>>>>>>> Hi Sergey,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> From the process point of view it's better to fix the issue
>>>>>>>>>>>>>> you've
>>>>>>>>>>>>>> found in another ticket. The failed test is not related 
>>>>>>>>>>>>>> to the
>>>>>>>>>>>>>> keystrokes caching.
>>>>>>>>>>>>>> So I suggest to push this fix as it is and file another
>>>>>>>>>>>>>> JIRA for
>>>>>>>>>>>>>> the keystrokes.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 7/22/2015 3:58 PM, Sergey Bylokhov wrote:
>>>>>>>>>>>>>>> Hi, Semyon.
>>>>>>>>>>>>>>> NPE occurs when we initialize KFM on the Toolkit thread, 
>>>>>>>>>>>>>>> but
>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>> is only a part of the bug, another issue is that we will 
>>>>>>>>>>>>>>> use
>>>>>>>>>>>>>>> cached keystrokes on the toolkit thread. But this
>>>>>>>>>>>>>>> keystrokes is
>>>>>>>>>>>>>>> bound to the appcontext so we should not use objects which
>>>>>>>>>>>>>>> connect to the application on the toolkit thread. This code
>>>>>>>>>>>>>>> should be carefully checked to remove appcontext related
>>>>>>>>>>>>>>> stuff
>>>>>>>>>>>>>>> from the toolkit thread.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 21.07.15 12:40, Semyon Sadetsky wrote:
>>>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Please review fix for JDK9:
>>>>>>>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8130895
>>>>>>>>>>>>>>>> webrev:
>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ssadetsky/8130895/webrev.00/
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> realSync() used in the test's TestRunnable class causes
>>>>>>>>>>>>>>>> events
>>>>>>>>>>>>>>>> come to the XAWT event loop but there are no any windows
>>>>>>>>>>>>>>>> created
>>>>>>>>>>>>>>>> at the moment and the system application context is not
>>>>>>>>>>>>>>>> initialized. This results in attempt to create the
>>>>>>>>>>>>>>>> KeyboardFocusManager instance on the XAWT's thread group
>>>>>>>>>>>>>>>> during
>>>>>>>>>>>>>>>> the XEvent dispatching. That in its turn causes NPE.
>>>>>>>>>>>>>>>> The solution: since KeyboardFocusManager should never be
>>>>>>>>>>>>>>>> instantiated in the toolkit event loop, the corresponding
>>>>>>>>>>>>>>>> check
>>>>>>>>>>>>>>>> was added.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -- 
>>>>>>>>>>>>>>> Best regards, Sergey.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> -- 
>>>>>>>>>>>>> Best regards, Sergey.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> -- 
>>>>>>>>>> Best regards, Sergey.
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>