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

[Semyon Sadetsky]

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.
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>