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.bylok...@oracle.com <mailto:sergey.bylok...@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.hal...@oracle.com <mailto:manajit.hal...@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.bylok...@oracle.com <mailto: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/


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



--
Best regards, Sergey.

Reply via email to