<AWT Dev> <AWT dev>[9] Review request for JDK-8165555: [macosx] VM crashes on second attempt to execute JCK interactive tests that use Robot (single JVM, agent)

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Thu Oct 6 10:50:22 UTC 2016


Looks fine.

On 05.10.16 12:57, Manajit Halder wrote:
> Hi Sergey,
>
> Thanks for the comments. Please review the updated webrev.
> http://cr.openjdk.java.net/~mhalder/8165555/webrev.04/
>
> The “frame” can’t be a local variable as it is used in two different
> threads inside the function.
>
> Thanks,
> Manajit
>
>> On 04-Oct-2016, at 6:26 pm, Sergey Bylokhov
>> <sergey.bylokhov at oracle.com <mailto:sergey.bylokhov at oracle.com>> wrote:
>>
>> Hi, Manajit.
>> It looksbetter, bet it can be improved a little bit.
>>  - you should not skip exceptions in the run();
>>  - It will be good to select the frame by mouse click before pressing
>> the key, otherwise you can run somthing else.
>>  - I suggest to iterate the code in main a few times in the loop(I
>> guess 10 iterations is ok)
>>  -The "frame" can be a local var inside the createRobot(), also the
>> method can be renamed.
>>
>> ----- manajit.halder at oracle.com <mailto:manajit.halder at oracle.com> wrote:
>> >
>> > Hi Sergey,
>> >
>> > Thanks for the review. Changes in CRobot.m are removed and added a
>> jtreg test case.
>> > Please review the webrev.
>> >
>> > http://cr.openjdk.java.net/~mhalder/8165555/webrev.03/
>> >
>> > Thanks,
>> > Manajit
>> >
>>
>>     > On 27-Sep-2016, at 4:15 pm, Sergey Bylokhov
>>     <Sergey.Bylokhov at oracle.com <mailto:Sergey.Bylokhov at oracle.com>>
>>     wrote:
>>
>>     > On 27.09.16 11:51, Manajit Halder wrote:
>>
>>         Hi Sergey,
>>
>>         Thanks for the comment. “self.javaToMacKeyMap is retaining the
>>         reference.
>>         Please review the modified code:
>>
>>         http://cr.openjdk.java.net/~mhalder/8165555/webrev.02/
>>
>>
>>     The changes in CRobot.m is unnecessary? I also suggest to write
>>     the test. Recently I have run all jck tests on my osx system and
>>     no of them were crashed, so it seems that the sequence of tests in
>>     the bug report are not stable and the new reg test will be welcome
>>     to catch such bugs always.
>>
>>             On 23-Sep-2016, at 10:24 pm, Sergey Bylokhov
>>             <Sergey.Bylokhov at oracle.com
>>             <mailto:Sergey.Bylokhov at oracle.com> <mailto:Sergey.Bylokhov at oracle.com>>
>>             wrote:
>>
>>             On 21.09.16 16:01, Manajit Halder wrote:
>>
>>                 Hi Sergey,
>>
>>                 Thanks for the comment.
>>
>>                 Access to "instance" is not broken. The problem is
>>                 with the dictionary
>>                 variable "javaToMacKeyMap" within the "instance"
>>                 reference.
>>                 The dictionary is not getting initialised for the
>>                 second time when
>>                 singleton method is called for the second time. The
>>                 dictionary is
>>                 getting initialised during the first time singleton
>>                 method is called and
>>                 hence crash was observed. Tried adding
>>                 self.javaToMacKeyMap
>>                 but doesn’t solve the problem.
>>
>>
>>             I still suggest you to check "self.javaToMacKeyMap" at
>>             line 48 this
>>             should call generated setter which will retain the reference.
>>
>>
>>                 I propose two solutions to this problem:
>>
>>                 Solution 1:
>>                 Reinitialise the dictionary every-time the singleton
>>                 method is called.
>>                 Webrev:
>>                 http://cr.openjdk.java.net/~mhalder/8165555/webrev.01/
>>
>>                 Drawback:
>>                 dictionary is initialised multiple times (every time
>>                 singleton method is
>>                 called).
>>
>>                 Solution 2:
>>                 Make the dictionary static.
>>                 Drawback:
>>                 Still dictionary need to be initialised multiple times.
>>                 No singleton method, just two static methods, one
>>                 method to initialise
>>                 the dictionary and other to get the key form the
>>                 dictionary.
>>                 Webrev:
>>                 Not prepared. Will be prepared if second solution
>>                 accepted.
>>
>>                 Test case:
>>                 There are no test cases. Problem can be reproduced by
>>                 executing following JCK test:
>>                 java -jar JCK-runtime-9/lib/jtjck.jar -mode:single
>>                 -k:interactive
>>                 "api/java_awt/interactive/event/EventTests.html#EventTest0019
>>                 api/java_awt/interactive/event/EventTests.html#EventTest0013”
>>
>>                 Thanks,
>>                 Manajit
>>
>>                     On 20-Sep-2016, at 4:42 am, Sergey Bylokhov
>>                     <Sergey.Bylokhov at oracle.com
>>                     <mailto:Sergey.Bylokhov at oracle.com>
>>                     <mailto:Sergey.Bylokhov at oracle.com>
>>                     <mailto:Sergey.Bylokhov at oracle.com>>
>>                     wrote:
>>
>>                     Hi, Manajit.
>>                     It seems that after the fix "(CRobotKeyCodeMapping
>>                     *) sharedInstance"
>>                     returns the new object per invocation, so it is
>>                     not really
>>                     sharedInstance. I am not sure I understand what is
>>                     wrong in the
>>                     current code, from the my point of view this is a
>>                     correct singleton.
>>                     It it true that the problem is in access to broken
>>                     "instance" and not
>>                     to "javaToMacKeyMap" inside the "instance"? If not
>>                     then
>>                     "javaToMacKeyMap" should be changed to
>>                     "self.javaToMacKeyMap".
>>                     Do you have a test case to reproduce the bug?
>>
>>                     On 19.09.16 15:26, Manajit Halder wrote:
>>
>>                         Hi All,
>>
>>                         Kindly review the fix for JDK9.
>>
>>                         Bug:
>>                         https://bugs.openjdk.java.net/browse/JDK-8165555
>>
>>                         Webrev:
>>                         http://cr.openjdk.java.net/~mhalder/8165555/webrev.00/
>>
>>                         Issue:
>>                         [macosx] VM crashes on second attempt to
>>                         execute JCK interactive tests
>>                         that use Robot (single JVM, agent)
>>
>>                         Cause:
>>                         While executing the JCK test for the second
>>                         time the robot was getting
>>                         initialised once again and old instance of
>>                         CRobotKeyCodeMapping was not
>>                         available.
>>                         Crash was observed while trying to access
>>                         invalid instance of
>>                         CRobotKeyCodeMapping.
>>
>>                         Fix:
>>                         A new instance of CRobotKeyCodeMapping is
>>                         created when robot is
>>                         initialised.
>>
>>                         Regards,
>>                         Manajit
>>
>>
>>
>>                     --
>>                     Best regards, Sergey.
>>
>>
>>
>>
>>             --
>>             Best regards, Sergey.
>>
>>
>>
>>
>>     --
>>     Best regards, Sergey.
>


-- 
Best regards, Sergey.


More information about the awt-dev mailing list