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

steve.x.northover at oracle.com steve.x.northover at oracle.com
Thu May 24 08:26:34 PDT 2012


Thanks so much for finding and fixing this critical issue.

Steve

On 24/05/2012 10:33 AM, 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 macosx-port-dev mailing list