HashedWheelTimer TimerTask may execute after call to Timeout.cancel()

[rzo]

Hello,

this seems good. I think introducing an atomic state field instead of 2 
boolean fields is better than synchronizing the methods.

thanks

-- Ron


On 12.08.2011 07:06, ??? (Trustin Lee) wrote:
> I actually realized what you intended to say.  Could you review my fix:
>
>   
> https://github.com/netty/netty/commit/dbb239203460253f84b0f848b8f05e3b6e11a2fd
>
> Cheers
>
> -- 
> Trustin Lee <http://gleamynode.net/>
>
> On Friday, August 12, 2011 at 1:46 PM, Trustin Lee wrote:
>
>> Hi Ron,
>>
>> I like the alternative 2. However, could you explain why the public
>> methods in HashedWheelTimeout should be synchronized? Is there
>> anything thread-unsafe there?
>>
>> Thanks
>>
>> On Tue, Aug 9, 2011 at 4:54 PM, <rzo at gmx.de <mailto:rzo at gmx.de>> wrote:
>>> Hello,
>>>
>>> given that the implementation is using "approximated times" a call 
>>> to Timeout.cancel() may not cancel the task.
>>>
>>> I think that the problem is:
>>>
>>> public void cancel() {
>>>            if (isExpired()) {
>>>                return;
>>>            }
>>>
>>>
>>> where isExpired checks:
>>>
>>> System.currentTimeMillis() > deadline
>>>
>>> Solution:
>>> Alternative 1:
>>>
>>> remove the check:
>>>
>>> if (isExpired()) {
>>>                return;
>>>            }
>>>
>>> Alternative 2:
>>>
>>> new field:
>>>
>>> boolean expired = false.
>>>
>>> this is set to true in:
>>>
>>> HashedWheelTimeout.expire()
>>>
>>> and isExpired()
>>> {
>>>   return canceled || expired;
>>> }
>>>
>>>
>>> I also propose that the public methods on HashedWheelTimeout be 
>>> synchronized.
>>>
>>> What do you think ?
>>>
>>> -- Ron
>>> http://sourceforge.net/projects/yajsw
>>> --
>>> NEU: FreePhone - 0ct/min Handyspartarif mit Geld-zurück-Garantie!
>>> Jetzt informieren: http://www.gmx.net/de/go/freephone
>>> _______________________________________________
>>> netty-users mailing list
>>> netty-users at lists.jboss.org <mailto:netty-users at lists.jboss.org>
>>> https://lists.jboss.org/mailman/listinfo/netty-users
>>
>>
>>
>> -- 
>> Trustin Lee, http://gleamynode.net/
>
>
>
> _______________________________________________
> netty-users mailing list
> netty-users at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/netty-users

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/netty-users/attachments/20110813/21d99d97/attachment.html