Yet another NIO networking API

Networking & Multiplayer
#1

well, how to justify this topic, let’s see…

there have been many NIO servers written and posted as examples on this board and elsewhere, but since so few of them included a NIO client, I decided to write my own.
I tried to keep the networking part as simple as possible, so what you basically get here is one server class, one client class and a listener interface to handle message arrival and reports. both, server and client have their own inbox, which you poll to retrieve received messages.

I included a simple program, that uses a server to synchronize it’s clients. please have a look and share your hate.

link << It actually works since 01 July 2006.

#2

best thing is, now I’m getting weird behaviour with the test app on my home computer, which I’m unable to reproduce on my work computer (where I wrote it).

moving the little thingies in a client too fast sends frequent network updates which causes something to break and the server hogs the cpu. how do you guys debug this networking stuff anyhow?

#3

hehe, debugging networking can definitely be a pain to mess with…particularly NIO stuff as I’m learning can be quite a beast to work with.

Just out of curiosity did you happen to look at http://javagamenetworking.dev.java.net in your searching? It provides very nice Client/Server features and should be about as simple as is possible without losing capabilities. I’m actually about to release version 2.0 which has some significant improvements and quite a bit of help from Riven setting me straight on my mistakes on NIO. :-p

I’ll take a look at your API and if something jumps out at me.

-Matt Hicks

#4

I haven’t looked at your code so this might be obvious to you, but have you made sure you are turning off write ops afer you are done writing (or aren’t trying to write data)? There are a few posts in these forums about that particular issue

#5

yes, I took a look at your code but it’s rather an overkill for my needs. simplicity in function and implementation is my personal goal with this one. pure TCP/IP reliable message delivery with a couple of method calls, basically to demonstrate the core functionality of NIO and make it work.

I also remember from another thread how Riven showed up and proclaimed your code to be buggy with some very obvious errors in it. just to mention an example, I’m using unsynchronized LinkedList-s in a very obvious manner for the sole reason that I’d love to be publicly chastised by Riven also. :slight_smile: what could go wrong with an unsynchronized LinkedList other than a poll returning null while another thread is adding an object? my code should be able to handle that but I’ll take a look at it today and synchronize them all.

if it’s something with the NIO stuff, than I’m kind of lost because it works, it just breaks very unpredictably.

believe me, I’ve read all those threads and then some more. you could take a look at the code, it’s very short! (and it’s Saturday) .

…you bring up a point though, because turning off write ops depends on one of those unsynchronized LinkedList accesse’s return value, so I should have considered that a clue earlier.

Update: so I’m using Vector for now. link

#6

Hm, there goes my reputation of fighting for worldpeace and what-not.

In NIOSession.doRead(…)

I found this piece of code:


        if (readState == 1)
        {
            if ((bytesLast = this.channel.read(bBuffer)) < 0) throw new IOException("ioe");
            else if ((bytesRead += bytesLast) == msgLength)
            {
                byte[] msgBody = new byte[msgLength];
                bBuffer.flip();
                bBuffer.get(msgBody);
                bBuffer.clear();
                readState = 0;
                bytesRead = 0;
                inbox.add(new NIOMessage(msgBody, selKey));
                listener.handleNIOMsg();
            }
        }

The problem here is that you might have received part of the next message too, in which case this is the case:

(bytesRead += bytesLast) > msgLength

To ensure you read-back the correct amount of bytes in your ByteBuffer, use something like this code:


if((bytesRead += bytesLast) > msgLength)
{
     byte[] msgBody = new byte[msgLength];

     // set 'range' to current message
     bb.position(0);
     bb.limit(msgLength);
     bb.get(msgBody);

     // set 'range' to pending bytes
     bb.position(msgLength);
     bb.limit(bytesRead);
     bb.compact();
}

I have to warn you though that the above code is not very efficient, but it will be fast enough for almost anything (will probably go over a few MB/s)

#7

And now a bug related to multi-threading:


            if (outbox.size() == 0)
            {
                ...
            }
            else
            {
                wBuffer[1] = outbox.remove(0);
                ...
            }

This code is not thread-safe! You seem to assume the outbox-size is not 0 when you arrive in the else-block. In reality another Thread might have snatched the object in between those two calls. The fact that Vector is synchronized is completely irrelevant, as the lock/monitor is lost between those calls. You’ll have to manually synchronize the two invocations.

Update:
On second thought it is unlikely that more than 1 thread accesses the doWrite method, so the problems you’re finding have nothing to do with this one.

#8

Praised Be The Riven!

seriously, thanks for the swift reply. guess what, you were right! best thing is I found an elegant solution right away. take a look:


        if (readState == 0)
        {
            if ((bytesLast = this.channel.read(lBuffer)) < 0) throw new IOException("ioe");
            else if ((bytesRead += bytesLast) == 2)
            {
                msgLength = lBuffer.getShort(0);
                lBuffer.clear();
                bBuffer.clear();
                bBuffer.limit(msgLength);
                readState = 1;
                bytesRead = 0;
            }
        }
        
        if (readState == 1)
        {
            if ((bytesLast = this.channel.read(bBuffer)) < 0) throw new IOException("ioe");
            else if ((bytesRead += bytesLast) == msgLength)
            {
                byte[] msgBody = new byte[msgLength];
                bBuffer.flip();
                bBuffer.get(msgBody);                
                readState = 0;
                bytesRead = 0;
                inbox.add(new NIOMessage(msgBody, selKey));
                listener.handleNIOMsg();
            }
        }

I just set the body Buffer’s limit on the fly before each read. it might not be too effective performance-wise if the messages come in small chunks, but that’s just a design decision which can be taken into consideration when assembling messages.

you are right, the MT-related bug you described shouldn’t be a bug really, because only the selector thread removes messages from the outbox. the worst that could happen is that the selector thread deletes the write ops just after another thread inserted a message to the outbox and set the channels key as writable. that’s very unlikely to occur and even if, it would get remedied with the next messasge inserted for sending.

I have updated all the links to the now hopefuly working code. thanks again.