Uploaded image for project: 'Traffic Server'
  1. Traffic Server
  2. TS-4522

Should signal SM with EVENT_ERROR on error in write_to_net_io(), EVENT_EOS only signaled from read_from_net()

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Patch Available
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: 7.1.0
    • Component/s: Core, Network
    • Labels:
      None

      Description

      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().

          if (!r || r == -ECONNRESET) {
      

      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:

        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;
      

      The 2nd Problem:

      In the iocore/net/I_NetVConnection.h, the comments for do_io_write:

      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;
      

      Only 3 Events was introduced

      • VC_EVENT_WRITE_READY
      • VC_EVENT_WRITE_COMPLETE
      • VC_EVENT_ERROR

      The code

      write_signal_done(VC_EVENT_EOS, nh, vc);

      should not be here (write_to_net_io).

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                oknet Chao Xu
                Reporter:
                oknet Chao Xu
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 2h 20m
                  2h 20m