<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)
Avik Niyogi
avik.niyogi at oracle.com
Thu Oct 6 12:12:04 UTC 2016
Looks good to me.
> On 06-Oct-2016, at 4:20 pm, Sergey Bylokhov <Sergey.Bylokhov at oracle.com> wrote:
>
> 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