[jira] [Work logged] (TS-4522) Should signal SM with EVENT_ERROR on error in write_to_net_io(), EVENT_EOS only signaled from read_from_net()

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

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

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

                Author: ASF GitHub Bot
            Created on: 01/Sep/16 06:30
            Start Date: 01/Sep/16 06:30
    Worklog Time Spent: 10m 
      Work Description: Github user oknet commented on the issue:

    https://github.com/apache/trafficserver/pull/701
  
    Copy&Paste bug in load_buffer_and_write() if 0 returned from write() or writev().


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

    Worklog Id:     (was: 27698)
    Time Spent: 50m  (was: 40m)

> Should signal SM with EVENT_ERROR on error in write_to_net_io(), EVENT_EOS only signaled from read_from_net()
> -------------------------------------------------------------------------------------------------------------
>
>                 Key: TS-4522
>                 URL: https://issues.apache.org/jira/browse/TS-4522
>             Project: Traffic Server
>          Issue Type: Bug
>          Components: Core, Network
>            Reporter: Oknet Xu
>            Assignee: Oknet Xu
>             Fix For: 7.0.0
>
>          Time Spent: 50m
>  Remaining Estimate: 0h
>
> The 1st Problem:
> The "r" saved return value from write(). The "r == 0" or "!r" is not means EOS. 
> Because of on a closed socket fd: 
> - read(socketfd) return 0
> - write(socketfd) return EPIPE
> In the write_to_net_io, we check the return value of write() with the same way to read().
> {code}
>     if (!r || r == -ECONNRESET) {
> {code}
> It is a copy & paste bug.
> The bug makes no VC_EVENT_EOS callbacked while write_to_net_io, but VC_EVENT_ERROR instead. 
> full code here:
> {code}
>   int64_t r = vc->load_buffer_and_write(towrite, buf, total_written, needs);
>   if (total_written > 0) {
>     NET_SUM_DYN_STAT(net_write_bytes_stat, total_written);
>     s->vio.ndone += total_written;
>   }
>   // check for errors
>   if (r <= 0) { // if the socket was not ready,add to WaitList
>     if (r == -EAGAIN || r == -ENOTCONN) {
>       NET_INCREMENT_DYN_STAT(net_calls_to_write_nodata_stat);
>       if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
>         vc->write.triggered = 0;
>         nh->write_ready_list.remove(vc);
>         write_reschedule(nh, vc);
>       }
>       if ((needs & EVENTIO_READ) == EVENTIO_READ) {
>         vc->read.triggered = 0;
>         nh->read_ready_list.remove(vc);
>         read_reschedule(nh, vc);
>       }
>       return;
>     }
>     if (!r || r == -ECONNRESET) {
>       vc->write.triggered = 0;
>       write_signal_done(VC_EVENT_EOS, nh, vc);
>       return;
>     }
>     vc->write.triggered = 0;
>     write_signal_error(nh, vc, (int)-total_written);
>     return;
> {code}
> The 2nd Problem:
> In the iocore/net/I_NetVConnection.h, the comments for do_io_write:
> {code}
> 257   /**
> 258     Initiates write. Thread-safe, may be called when not handling
> 259     an event from the NetVConnection, or the NetVConnection creation
> 260     callback.
> 261 
> 262     Callbacks: non-reentrant, c's lock taken during callbacks.
> 263 
> 264     <table>
> 265       <tr>
> 266         <td>c->handleEvent(VC_EVENT_WRITE_READY, vio)</td>
> 267         <td>signifies data has written from the reader or there are no bytes available for the reader to write.</td>
> 268       </tr>
> 269       <tr>
> 270         <td>c->handleEvent(VC_EVENT_WRITE_COMPLETE, vio)</td>
> 271         <td>signifies the amount of data indicated by nbytes has been read from the buffer</td>
> 272       </tr>
> 273       <tr>
> 274         <td>c->handleEvent(VC_EVENT_ERROR, vio)</td>
> 275         <td>signified that error occured during write.</td>
> 276       </tr>
> 277     </table>
> 278 
> 279     The vio returned during callbacks is the same as the one returned
> 280     by do_io_write(). The vio can be changed only during call backs
> 281     from the vconnection. The vconnection deallocates the reader
> 282     when it is destroyed.
> 283 
> 284     @param c continuation to be called back after (partial) write
> 285     @param nbytes no of bytes to write, if unknown msut be set to INT64_MAX
> 286     @param buf source of data
> 287     @param owner
> 288     @return vio pointer
> 289 
> 290   */
> 291   virtual VIO *do_io_write(Continuation *c, int64_t nbytes, IOBufferReader *buf, bool owner = false) = 0;
> {code}
> Only 3 Events was introduced
> - VC_EVENT_WRITE_READY
> - VC_EVENT_WRITE_COMPLETE
> - VC_EVENT_ERROR
> The code {code}write_signal_done(VC_EVENT_EOS, nh, vc);{code} should not be here (write_to_net_io).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)