[ 
https://issues.apache.org/jira/browse/TS-4614?focusedWorklogId=25515&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-25515
 ]

ASF GitHub Bot logged work on TS-4614:
--------------------------------------

                Author: ASF GitHub Bot
            Created on: 15/Jul/16 21:41
            Start Date: 15/Jul/16 21:41
    Worklog Time Spent: 10m 
      Work Description: Github user shinrich commented on a diff in the pull 
request:

    https://github.com/apache/trafficserver/pull/766#discussion_r71044678
  
    --- Diff: iocore/net/UnixNetVConnection.cc ---
    @@ -1139,8 +1139,8 @@ UnixNetVConnection::mainEvent(int event, Event *e)
           (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));
    +#endif
         return EVENT_CONT;
    --- End diff --
    
    This appears to be the more critical fix.  It looks like a standard 
reschedule if you don't get the lock.  But it appears that we only do this if 
the event is an active timeout (if INACTIVITY_TIMEOUT is set).  It seems that 
we are not currently building with INACTIVITY_TIMEOUT defined (at least I'm 
not).  So this change would cause the reschedule to never happen.  So if you 
don't get the lock the first time, you will never process the event.
    
    I'm not following the description in the JIRA.  Is the concern that we are 
double freeing an event object?  


Issue Time Tracking
-------------------

            Worklog Id:     (was: 25515)
            Time Spent: 10m
    Remaining Estimate: 0h

> 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
>            Assignee: Oknet Xu
>             Fix For: 7.0.0
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> 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)

Reply via email to