<AWT Dev> [7u6] Please review my fix for 7171163: [macosx] Shortcomings in the design of the secondary native event loop made JavaFX DnD deadlock

Artem Ananiev artem.ananiev at oracle.com
Thu May 24 09:12:14 PDT 2012


Hi, Alex,

the fix looks fine for 7u6. In JDK8 version of the fix, I would rename 
startNativeNestedEventLoop() in LWCToolkit to something like runOneEvent().

Thanks,

Artem

On 5/24/2012 6:33 PM, Alexander Zuev wrote:
> Steve,
>
> i guess you are right and i gladly borrow your version of comment to use.
> This and two other typos caught by Scott Palmer leads us to the fourth
> version of
> the webrev which can be found here:
> http://cr.openjdk.java.net/~kizune/7171163/webrev.03
>
> With best regards,
> Alex
>
> On 5/24/12 17:48, steve.x.northover at oracle.com wrote:
>> Thanks for commenting this for the next guy.
>>
>> Nit pick:
>>
>> // Despite the naming this method on MacOS only making one event from
>> // the native loop to be executed so this call is not blocking
>>
>> Is not great English. How about "Despite the naming of this method, on
>> MacOS only one event is read and dispatched before this method
>> returns. This call is non-blocking and does not wait for an event"
>>
>> Thanks,
>> Steve
>>
>> On 24/05/2012 5:35 AM, Alexander Zuev wrote:
>>> Steve,
>>>
>>> i've updated comments anywhere i can to reflect specific design of
>>> this feature on Mac.
>>> Also i have removed the unneeded call to the native function from the
>>> exit() method to save
>>> us some cycles on Java<->JNI call.
>>>
>>> Updated webrev can be found here:
>>> http://cr.openjdk.java.net/~kizune/7171163/webrev.01/
>>>
>>> I also will send the formal request to approve the same change in
>>> jdk8 since we have to push change
>>> there before we can push it into the jdk7u6.
>>>
>>> With best regards,
>>> Alex
>>>
>>> On 5/23/12 22:26, steve.x.northover at oracle.com wrote:
>>>> ToolkitThreadBlockedHandler seems to be shared code and it follows
>>>> the enter / exit naming that is wrong. It seems that a lot of
>>>> changes will be needed in shared code so I'm not sure the renaming
>>>> is worth it.
>>>>
>>>> You could add update the comments to indicate how this is used and
>>>> save the renaming for another day.
>>>>
>>>> Steve
>>>>
>>>> On 23/05/2012 2:11 PM, Alexander Zuev wrote:
>>>>> Steve,
>>>>>
>>>>> you are absolutely right - this native method is used only by our
>>>>> implementation of the DnD so
>>>>> i may just rename it and the only reason i have not done it is to
>>>>> minimize the scope of the change.
>>>>> So if you think renaming the method is required i will gladly do it.
>>>>>
>>>>> With best regards,
>>>>> Alex
>>>>>
>>>>> On 5/23/12 21:21, steve.x.northover at oracle.com wrote:
>>>>>> The implementation matches how these methods are used by drag and
>>>>>> drop but the naming makes no sense. Who else calls these methods?
>>>>>> I'm assuming nobody (or that they are all called with the same
>>>>>> implementation in mind).
>>>>>>
>>>>>> startNativeNestedEventLoop is really "read and dispatch an event
>>>>>> without blocking".
>>>>>>
>>>>>> Steve
>>>>>>
>>>>>> On 23/05/2012 12:51 PM, Alexander Zuev wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> please review my fix for the
>>>>>>> 7171163: [macosx] Shortcomings in the design of the secondary
>>>>>>> native event loop made JavaFX DnD deadlock
>>>>>>>
>>>>>>> Bug can be found at
>>>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7171163
>>>>>>>
>>>>>>> Webrev can be found at:
>>>>>>> http://cr.openjdk.java.net/~kizune/7171163/webrev.00/
>>>>>>>
>>>>>>> With best regards,
>>>>>>> Alexander Zuev
>>>>>
>>>
>



More information about the awt-dev mailing list