Commons Net
  1. Commons Net
  2. NET-437

TelnetInputStream doesn't support non-blocking IO when reader thread is not enabled

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.1
    • Fix Version/s: 3.1
    • Component/s: Telnet
    • Labels:
      None
    • Environment:

      Java 6 + commons-net-3.0.1

      Description

      When the telnet client is used without allowing it to create it's own reader thread (i.e. setReaderThread(false)) then the TelnetInputStream.available() method will always return 0 bytes available. This makes non-blocking IO impossible as you need to actualy call read to get the data without knowing if it will block or not.

      This fix to the available method in org.apache.commons.net.telnet.TelnetInputStream, seems to fix the issue, and should work for reader threads as well:

       
          @Override
          public int available() throws IOException
          {
              // Critical section because run() may change __bytesAvailable
              synchronized (__queue)
              {
              	if (__bytesAvailable == 0 && !__threaded) {
              		return super.available();
              	} else {
              		return __bytesAvailable;
              	}
              }
          }
      

        Activity

        Hide
        Sebb added a comment -

        I'm not quite sure why it's important to check __bytesAvailable?

        Perhaps the condition should be coded as follows:

        if (__threaded) {
            return __bytesAvailable;
        } else {
            return __bytesAvailable + super.available();
        }
        

        This should return a value closer to the actual number available.

        Show
        Sebb added a comment - I'm not quite sure why it's important to check __bytesAvailable? Perhaps the condition should be coded as follows: if (__threaded) { return __bytesAvailable; } else { return __bytesAvailable + super .available(); } This should return a value closer to the actual number available.
        Hide
        Gavin Camp added a comment -

        Well for the non-threaded case __bytesAvailable will always be zero (this is the bug). I was trying to check to make sure they hadn't spawned their own thread for the reader. Although i'm not sure if this is easily possible (perhaps through class inheritance). I guess if you don't want this case the code would be:

         
        if (__threaded) {
            return __bytesAvailable;
        } else {
            return super.available();
        }
        
        Show
        Gavin Camp added a comment - Well for the non-threaded case __bytesAvailable will always be zero (this is the bug). I was trying to check to make sure they hadn't spawned their own thread for the reader. Although i'm not sure if this is easily possible (perhaps through class inheritance). I guess if you don't want this case the code would be: if (__threaded) { return __bytesAvailable; } else { return super.available(); }
        Hide
        Sebb added a comment -

        Or even just do the same for both:

        return __bytesAvailable + super.available();
        

        AFAICT, that should always work, and would be more accurate.

        Show
        Sebb added a comment - Or even just do the same for both: return __bytesAvailable + super .available(); AFAICT, that should always work, and would be more accurate.
        Hide
        Gavin Camp added a comment -

        Sure sounds good.

        Show
        Gavin Camp added a comment - Sure sounds good.
        Hide
        Sebb added a comment -

        Fixed in SVN.

        I've uploaded a SNAPSHOT build to

        https://repository.apache.org/content/repositories/snapshots/commons-net/commons-net/3.1-SNAPSHOT/

        if you want to try it out.

        Show
        Sebb added a comment - Fixed in SVN. I've uploaded a SNAPSHOT build to https://repository.apache.org/content/repositories/snapshots/commons-net/commons-net/3.1-SNAPSHOT/ if you want to try it out.

          People

          • Assignee:
            Unassigned
            Reporter:
            Gavin Camp
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 1h
              1h
              Remaining:
              Remaining Estimate - 1h
              1h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development