<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