[
https://issues.apache.org/jira/browse/TS-4614?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15355214#comment-15355214
]
ASF GitHub Bot commented on TS-4614:
------------------------------------
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/766#discussion_r68942583
--- Diff: iocore/net/UnixNet.cc ---
@@ -682,8 +682,9 @@ NetHandler::_close_vc(UnixNetVConnection *vc,
ink_hrtime now, int &handle_event,
// create a dummy event
Event event;
event.ethread = this_ethread();
- vc->handleEvent(EVENT_IMMEDIATE, &event);
- ++handle_event;
+ if (vc->handleEvent(EVENT_IMMEDIATE, &event) == EVENT_DONE) {
+ ++handle_event;
+ }
}
}
--- End diff --
I guess this makes a bit more sense. As far as I can tell the handle_event
variable is only used for a Debug statement, so the change isn't crucial one
way or another.
> In UnixNetVConnection::mainEvent should not do e->schedule_in for dummy event
> callback
> ---------------------------------------------------------------------------------------
>
> Key: TS-4614
> URL: https://issues.apache.org/jira/browse/TS-4614
> Project: Traffic Server
> Issue Type: Bug
> Components: Cop
> Reporter: Oknet Xu
>
> NetHandler has a method: _close_vc ， It is called by InactivityCop.
> first, create a dummy Event in stack,
> then call UnixNetVConnection::mainEvent by vc->handleEvent(EVENT_IMMEDIATE,
> &event);
> the handleEvent is mainEvent here.
> In the UnixNetVConnection::mainEvent code:
> ```
> int
> UnixNetVConnection::mainEvent(int event, Event *e)
> {
> ink_assert(event == EVENT_IMMEDIATE || event == EVENT_INTERVAL);
> ink_assert(thread == this_ethread());
> MUTEX_TRY_LOCK(hlock, get_NetHandler(thread)->mutex, e->ethread);
> MUTEX_TRY_LOCK(rlock, read.vio.mutex ? read.vio.mutex : e->ethread->mutex,
> e->ethread);
> MUTEX_TRY_LOCK(wlock, write.vio.mutex ? write.vio.mutex :
> e->ethread->mutex, e->ethread);
> if (!hlock.is_locked() || !rlock.is_locked() || !wlock.is_locked() ||
> (read.vio.mutex && rlock.get_mutex() != read.vio.mutex.get()) ||
> (write.vio.mutex && wlock.get_mutex() != write.vio.mutex.get())) {
> #ifdef INACTIVITY_TIMEOUT
> if (e == active_timeout)
> #endif
> e->schedule_in(HRTIME_MSECONDS(net_retry_delay));
> return EVENT_CONT;
> }
> ```
> the dummy Event would be schedule_in into Event System by
> e->schedule_in(HRTIME_MSECONDS(net_retry_delay));
> I think we should move the schedule_in into the INACTIVITY_TIMEOUT macro.
> ```
> #ifdef INACTIVITY_TIMEOUT
> if (e == active_timeout)
> e->schedule_in(HRTIME_MSECONDS(net_retry_delay));
> #endif
> ```
> I'm try to allocate a Event instead dummy Event, but meet Event System
> callback on a deallocated UnixNetVConnection.
> due to NetHandler called close_UnixNetVConnection before Event System
> callback the Event by schedule_in.
> In NetHandler::_close_vc, depend the return value (EVENT_DONE or EVENT_CONT)
> from UnixNetVConnection::mainEvent, to do ++handle_event; or not.
> ```
> if (vc->handleEvent(EVENT_IMMEDIATE, &event) == EVENT_DONE)
> ++handle_event;
> ```
> the 3 MUTEX_TRY_LOCK not always success on InactivityCop callback,
> due to the mutex of ServerSessionVC may different from ClientSessionVC.
> Only mutex of ServerSession is set to HttpSM when HttpSM pick up a Server
> Session from SessionPool. ServerSessionVC still keep the old mutex.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
