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/ <http://cr.openjdk.java.net/~mhalder/8165555/webrev.03/> Thanks, Manajit > On 27-Sep-2016, at 4:15 pm, Sergey Bylokhov <sergey.bylok...@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/ >> <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.bylok...@oracle.com <mailto:sergey.bylok...@oracle.com> >>> <mailto:sergey.bylok...@oracle.com <mailto:sergey.bylok...@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/ >>>> <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.bylok...@oracle.com <mailto:sergey.bylok...@oracle.com> >>>>> <mailto:sergey.bylok...@oracle.com <mailto:sergey.bylok...@oracle.com>> >>>>> <mailto:sergey.bylok...@oracle.com <mailto:sergey.bylok...@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.
