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

[Sergey Bylokhov]

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


-- 
Best regards, Sergey.