CODEWITHSUNDEEP
javasocketsnetwork-programmingstream
noisy cat
noisy cat

Reputation: 3065

Java socket - lost bytes and 100% processor power consumption

Im making a simple socket app that connects with my server using through TCP. I need to read 2 byte values sometimes, so it all goes like:

public byte[] read(int bytes)
{
    byte b[] = new byte[bytes];
    try {
        in.read(b); //in is InputStream from properly connected Socket.getInputStream()
    return b;
    } catch (IOException e) {
        return null;
    }
}

This function should receieve given number of bytes and return it in array. The problem is, that sometimes it reads one byte before rest is available and returns weird data.

byte a[]=read(2); //Program is blocked here untill some bytes arrive...
System.out.prntln(a[0]); //always correct
System.out.prntln(a[1]); //unprintable character (probably 0 or -1)

My quick fix was to add while loop checking if there is enough data to read:

public byte[] read(int bytes)
{
    byte b[] = new byte[bytes];
    try {
        while (in.available()<bytes); //It does the thing
        in.read(b);
    return b;
    } catch (IOException e) {
        return null;
    }
}

But that loop is using 100% of processor power (one core, actually), which is very annoying. Is there any way to rewrite that function (param and return value must be exactly the same) so it works correctly?

Thanx in advance :)

Upvotes: 2

Views: 227

Answers (2)

Stephen C
Stephen C

Reputation: 718836

Yes. Your quick fix is a bad idea. In fact, any code that uses available is probably broken.

Here's how I'd do this:

public byte[] read(int bytes) throws IOException {
    byte b[] = new byte[bytes];
    int pos = 0;
    while (pos < b.length) {
       int nosRead = in.read(b, pos, b.length - pos);
       if (nosRead == -1) {
          return null;  // Not the best idea ... see below.
       } else {
          pos += nosRead;
       }
    }
    return b;
}

There is no need to "poll" the stream using available(). There is no need to sleep. You just make use of the fact that read will only return if either it has read at least one byte OR you've fit the "end of stream".

Note that I also don't think it is right to return a null if your read method fails. It is better to throw an exception ... or let the IOException from the in.read() call propagate.

This applies to my rewrite as well ... I didn't want to change the behaviour of your code in that respect.

Upvotes: 7

user207421
user207421

Reputation: 310915

Throw it away and use DataInputStream.readFully().

Upvotes: 2

Related Questions