Uploaded image for project: 'HPCC'
  1. HPCC
  2. HPCC-22103

Suspect code when handling exception within receiveDaFsBuffer in dafilesrv client.

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 7.8.40, 7.10.14
    • Component/s: dafilesrv
    • Labels:
      None

      Description

      Within the function below, if it generates a DAFSERR_protocol_failure exception , it calls flushDaFsSocket with exception handlers to try to clear up the socket.

      It would be safer to move the handling outside of the exception handlers.
      flushDaFsSocket also seems a bit odd, sleeping for 1 second ever (max) 1k it reads.

      We have seen the client get stuck in this code seemingly looping indefinitely, we're not sure how much it was reading each time, but it had been running for ~24hrs.
      It looks like it should break out if min_size is not read.

      It should probably at least have a max timeout and close the socket.

      void receiveDaFsBuffer(ISocket * socket, MemoryBuffer & tgt, unsigned numtries, size32_t maxsz)
          // maxsz is a guess at a resonable upper max to catch where protocol error
      {
          sRFTM tm(maxReceiveTime);
          size32_t gotLength = receiveDaFsBufferSize(socket, numtries,tm.timemon);
          if (gotLength) {
              size32_t origlen = tgt.length();
              try {
                  if (gotLength>maxsz) {
                      StringBuffer msg;
                      msg.appendf("receiveBuffer maximum block size exceeded %d/%d",gotLength,maxsz);
                      PrintStackReport();
                      throw createDafsException(DAFSERR_protocol_failure,msg.str());
                  }
                  unsigned timeout = SERVER_TIMEOUT*(numtries?numtries:1);
                  if (tm.timemon) {
                      unsigned remaining;
                      if (tm.timemon->timedout(&remaining)||(remaining<10))
                          remaining = 10;
                      if (remaining<timeout)
                          timeout = remaining;
                  }
                  size32_t szread;
                  SOCKREADTMS(socket)((gotLength<4000)?tgt.reserve(gotLength):tgt.reserveTruncate(gotLength), gotLength, gotLength, szread, timeout);
              }
              catch (IJSOCK_Exception *e) {
                  if (e->errorCode()!=JSOCKERR_timeout_expired) {
                      EXCLOG(e,"receiveDaFsBuffer(1)");
                      PrintStackReport();
                      if (!tm.timemon||!tm.timemon->timedout())
                          flushDaFsSocket(socket);
                  }
                  else {
                      EXCLOG(e,"receiveDaFsBuffer");
                      PrintStackReport();
                  }
                  tgt.setLength(origlen);
                  throw;
              }
              catch (IException *e) {
                  EXCLOG(e,"receiveDaFsBuffer(2)");
                  PrintStackReport();
                  if (!tm.timemon||!tm.timemon->timedout())
                      flushDaFsSocket(socket);
                  tgt.setLength(origlen);
                  throw;
              }
      
          }
          tgt.setEndian(__BIG_ENDIAN);
      }
      

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                mckellyln Mark Kelly
                Reporter:
                jakesmith Jake Smith
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: