[
https://issues.apache.org/jira/browse/TS-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13588535#comment-13588535
]
John Plevyak commented on TS-1405:
----------------------------------
There is still a nasty race condition. The call to set_event_cancel() is
async, so it can happen at any time. It first sets the "cancelled" flag then
it enqueues in the atomic list. Between these two events EThread::execute can
pull it from the external queue, check the flag, call free_event(). If this
happens, the atomic list insert will result in a clobber of free'd memory.
Instead, the only place that the event can be free'd when "cancelled" is in
process_cancel_event() which should probably be named
process_cancelled_events().
Also, no atomic is required to set the cancelled flag. This flag can only be
set while holding the mutex of the Event, so it is single threaded (there can
be no race). I would suggest adding an ink_debug_assert() to that effect, but
not using an atomic. It isn't strictly wrong, but it gives the impression
that the lock isn't required which it most definitely is. Without the lock
there would be a race inside process_event which takes the lock, checks
cancelled and the runs the event. If it was possible to cancel without the
lock, it could happen between the check and the running of the event which
would be "very bad".
What do you think Bin Chen? I can update the patch or do you want to
incorporate my comments?
> apply time-wheel scheduler about event system
> ----------------------------------------------
>
> Key: TS-1405
> URL: https://issues.apache.org/jira/browse/TS-1405
> Project: Traffic Server
> Issue Type: Improvement
> Components: Core
> Affects Versions: 3.2.0
> Reporter: Bin Chen
> Assignee: Bin Chen
> Fix For: 3.3.1
>
> Attachments: linux_time_wheel.patch, linux_time_wheel_v2.patch,
> linux_time_wheel_v3.patch, linux_time_wheel_v4.patch
>
>
> when have more and more event in event system scheduler, it's worse. This is
> the reason why we use inactivecop to handler keepalive. the new scheduler is
> time-wheel. It's have better time complexity(O(1))
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
