Ok... I'm just baffled. I've got a nice little networked game put together... works great on a LAN. But as soon as someone tries to connect over the internet, it locks the client and server hard. What's going on? I'm using non-blocking IO with TCP only.

Any ideas? I'm lost.

BTW, I tried only sending so many packets per second... as well as increasing my buffer size on the server.

Well, I'm desperate. Turns out, I keep getting an outofmemory error in my server thread as soon as someone connects from the internet. So, if someone could please tell me what I'm doing wrong, I'd appreciate it. Here's the code:

package networkers;

import java.io.IOException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
import java.nio.channels.ClosedChannelException;
import java.nio.channels.SelectionKey;
import java.nio.channels.Selector;
import java.nio.channels.ServerSocketChannel;
import java.nio.channels.SocketChannel;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.Set;

public class TCPServer extends Thread {
private static final int BUFFER_SIZE = 16000;
private static final int PORT = 10997;

private ServerSocketChannel sSockChan;
private Selector readSelector;
public boolean running;
private LinkedList clients;
private ByteBuffer readBuffer;
private ByteBuffer writeBuffer;
//private CharsetDecoder asciiDecoder;

private InetAddress serveraddress;

private boolean acceptingconnections = true;

private int howmanyplayers = 0;
private int playerpointer = 0;
private int maxplayers = 4;

private InetAddress[] players = new InetAddress[maxplayers];

public TCPServer(InetAddress addrpass) {
serveraddress = addrpass;
clients = new LinkedList();
readBuffer = ByteBuffer.allocate(BUFFER_SIZE);
writeBuffer = ByteBuffer.allocate(BUFFER_SIZE);
//asciiDecoder = Charset.forName( "US-ASCII").newDecoder();

for (int i=0;i<players.length;i++) {
players[i] = null;
}


}

private void initServerSocket() {
try {
// open a non-blocking server socket channel
sSockChan = ServerSocketChannel.open();
sSockChan.configureBlocking(false);
// bind to localhost on designated port
sSockChan.socket().bind(new InetSocketAddress(serveraddress, PORT));
System.out.println("Server started on " + serveraddress + ":" + PORT);

// get a selector for multiplexing the client channels
readSelector = Selector.open();

sSockChan.register(readSelector,SelectionKey.OP_ACCEPT);
}
catch (Exception e) {
System.out.println("error initializing server: " + e);
}
}

public void run() {
System.out.println("Starting server...");
Thread.currentThread().setName("Server Thread");
initServerSocket();

running = true;

// block while we wait for a client to connect
while (running) {
// check for new client connections
if (acceptingconnections) {
acceptNewConnections();
}

// check for incoming mesgs
readIncomingPackets();

// sleep a bit
Thread.yield();
}

try {
readSelector.close();
sSockChan.socket().close();
sSockChan.close();
} catch (IOException e) {
e.printStackTrace();
}

}

private void acceptNewConnections() {
try {
SocketChannel clientChannel;

// since sSockChan is non-blocking, this will return immediately
// regardless of whether there is a connection available
while ((clientChannel = sSockChan.accept()) != null) {
howmanyplayers = 0;
playerpointer = -1;
for (int i=0;i<players.length;i++) {
if (players[i] != null) {
howmanyplayers++;
} else if (playerpointer == -1){
playerpointer = i;
}
}

System.out.println("Client found!");
System.out.println("Client Added");
System.out.println("got connection from: " + clientChannel.socket().getInetAddress());
NetEvent tempevent = new NetEvent(-1,0,"login from: " + clientChannel.socket().getInetAddress());
sendBroadcastPacket(tempevent, clientChannel);
tempevent.eventType = 7;
tempevent.data = String.valueOf(playerpointer);
sendBroadcastPacket(tempevent, clientChannel);

addNewClient(clientChannel);

tempevent.eventType = 0;
tempevent.data = "Welcome to SpaceOps, there are " + clients.size() + " users online.";
sendPacket(clientChannel, tempevent);
tempevent.data = "You are Player " + playerpointer;
sendPacket(clientChannel, tempevent);
tempevent.data = "Press ESC to exit";
sendPacket(clientChannel, tempevent);
NetEvent newplayer = new NetEvent(-1,8,String.valueOf(playerpointer));
sendPacket(clientChannel,newplayer);
System.out.println("Welcome message sent");

for (int i=0; i<players.length; i++) {
if (players[i] != null) {
//if (players[i] != clientChannel.socket().getInetAddress()) {
tempevent.eventType = 7;
tempevent.data = String.valueOf(i);
sendPacket(clientChannel, tempevent);
//}
}
}
System.out.println("Initial Sync Complete!");

if (howmanyplayers >= maxplayers) {
acceptingconnections = false;
}
}
}
catch (IOException ioe) {
System.out.println("error during accept(): " + ioe);
}
catch (Exception e) {
System.out.println("exception in acceptNewConnections()" + e);
}
}

private void readIncomingPackets() {
try {
// non-blocking select, returns immediately regardless of how many keys are ready
readSelector.selectNow();

// fetch the keys
Set readyKeys = readSelector.selectedKeys();

// run through the keys and process
Iterator i = readyKeys.iterator();
while (i.hasNext()) {
SelectionKey key = (SelectionKey) i.next();
i.remove();
SocketChannel channel = (SocketChannel) key.channel();
readBuffer.clear();

// read from the channel into our buffer
long nbytes;
try {
nbytes = channel.read(readBuffer);
} catch (IOException ie) {
nbytes = -1;
}

// check for end-of-stream
if (nbytes == -1) {
removeClient(channel);
} else {
while (readBuffer.hasRemaining()) {
readBuffer.flip( );
sendBroadcastRawPacket(readBuffer, channel);
}
readBuffer.clear( );
}

}
} catch (Exception e) {
}

}

private void removeClient(SocketChannel channel) {
//// Which Player? ////
int whichplayer = -1;
for (int i=0;i<players.length;i++) {
if (players[i] != null) {
if (players[i] == channel.socket().getInetAddress()) {
whichplayer = i;
}
}
}
players[whichplayer] = null;
try {
System.out.println("disconnect: " + channel.socket().getInetAddress() + ", end-of-stream");
channel.close();
clients.remove(channel);
NetEvent tempevent = new NetEvent(-1,0,"");
tempevent.data = "logout: " + channel.socket().getInetAddress();
sendBroadcastPacket(tempevent, channel);
tempevent.eventType = 9;
tempevent.playerID = whichplayer;
tempevent.data = String.valueOf(whichplayer);
sendBroadcastPacket(tempevent, channel);
} catch (IOException e) {
}
}

private void addNewClient(SocketChannel chan) {
// add to our list
/*
try {
System.out.println(chan.socket().getTcpNoDelay());
System.out.println(chan.socket().getKeepAlive());
} catch (SocketException e) {
e.printStackTrace();
}
*/

// register the channel with the selector
chan.socket().setPerformancePreferences(0,2,1);
try {
chan.configureBlocking( false);
chan.register(readSelector, SelectionKey.OP_READ);
}
catch (ClosedChannelException cce) {
System.out.println("Channel Closed!");
}
catch (IOException ioe) {
System.out.println("IOException!");
}

players[playerpointer] = chan.socket().getInetAddress();

clients.add(chan);
}

private void sendPacket(SocketChannel channel, NetEvent event) {
prepWriteBuffer(event);
channelWrite(channel, writeBuffer);
}

private void sendBroadcastPacket(NetEvent event, SocketChannel from) {
prepWriteBuffer(event);
Iterator i = clients.iterator();
while (i.hasNext()) {
SocketChannel channel = (SocketChannel)i.next();
//if (channel != from)
channelWrite(channel, writeBuffer);
}
}

private void sendBroadcastRawPacket(ByteBuffer thisbuffer, SocketChannel from) {
Iterator i = clients.iterator();
while (i.hasNext()) {
SocketChannel channel = (SocketChannel)i.next();
//if (channel != from)
channelWrite(channel, thisbuffer);
}
}


private void prepWriteBuffer(NetEvent event) {
// fills the buffer from the given string
// and prepares it for a channel write
writeBuffer.clear();
writeBuffer.putInt(event.data.getBytes().length);
writeBuffer.putInt(event.gameID);
writeBuffer.putInt(event.playerID);
writeBuffer.putInt(event.eventType);
writeBuffer.put(event.data.getBytes());
writeBuffer.flip();
}

private void channelWrite(SocketChannel channel, ByteBuffer writeBuffer) {
long nbytes = 0;
long toWrite = writeBuffer.remaining();

if (channel.isConnected()) {
// loop on the channel.write() call since it will not necessarily
// write all bytes in one shot
try {
while (nbytes != toWrite) {
nbytes += channel.write(writeBuffer);
}
}

catch (ClosedChannelException cce) {
System.out.println("Closed Channel Exception!");
}
catch (IOException e) {
System.out.println("Socket Closed while still writing buffer!");
removeClient(channel);
//writeBuffer.clear();
}
catch (Exception e) {
System.out.println("TCPSERVER-Exception!");
e.printStackTrace();
}
} else {
removeClient(channel);
}

// get ready for another write if needed
writeBuffer.rewind();
}

public void stopNewConnects() {
acceptingconnections = false;
}

public void shutdown() {
running = false;
}

}



which line does it choke on ?

It doesn't. It ends up with an outofmemory error on the thread.

I lowered the number of packets that clients were sending to a huge extent, and also setup the server to do nothing but relay the raw packet data to all the clients, which seemed to resolve the outofmemory error - only problem is though, now clients are pretty choppy. I'm trying to write an action game (top down space-ship shooter style), but am not sure how to make it smooth using TCP sockets.

I think the problem I'm getting is that when someone on the internet connects, the TCP packets are getting backed up in the buffer as they're transmitted over the line and are then verified. Is there a way to say "don't bother verifying?" Kind of a fire-and-forget packet? That's why I was thinking about using UDP instead... but it's proving complicated to send some packets as UDP and others as TCP (mostly because I just don't know how to do both channel types on the same server).

Any ideas?

The out of memory error is probably because of broken packet parsing. You read in the length of the packet and use it to create an array. If that number is not what was sent you'll end up allocating a gigabyte of memory.

Can be related to the way you read packets. How do you know you've read a whole packet and not only a few bytes? Remember that a tcp socket is just a stream of data.

Doing on a networked game on the internet can be very difficult. Some games are impossible to do because of latency. There was a good thread  on javagaming called Physics Collisions + Network Latency that had some good points.

Be sure you don't flood the line. Estimate how many bytes you send every second and compare it to the internet line. Also, you must monitor how much data the reciever has recieved and pause if there is a blockage. etc. etc.

hmmm... very good points. How should you read the packets then to verify that they're not broken? (and discard them if they are?) What you're saying sounds very much like what might be happening, as I modified the above code (and updated it on the page) to simply relay incoming packets back out to all the connected clients - once I did that, I started getting the outofmemory error on the client side.

I've searched all over looking for a good tutorial on "proper" NIO networking code, but haven't found anything. The above code is just a heavily modified version of some code from the Java Gaming Book by Brackeen (which has a lot of good concepts in it, but leaves a lot out for more advanced stuff).

you could have fixed size header (+ 2 byte checksum) - that way you can avoid the issue

I do have a fixed header size... I just figured I didn't need it on the server now since I'm just taking data and passing it along.

I'm starting to wonder if I just need to create separate packet queues on the server for each client in order to control the number of packets sent to everyone (fewer for people with lower bandwidth). Of course, that means doing most of my processing on the server side.

You have to wait until you know a whole packet is read. Here is some code from a server a wrote a while back.


} else if (key.isReadable()) {
// read
ReadableByteChannel rbc = (ReadableByteChannel) key.channel();
Connection connection = (Connection) key.attachment();

int numBytesRead = -1;
try {
numBytesRead = rbc.read(connection.inputBuffer);
if (numBytesRead < 0) {
// end-of-stream
key.cancel();
removeConnection(connection);
} else {
connection.onBytesRead();
}
} catch (IOException e) {
key.cancel();
removeConnection(connection);
}



Noticed that every client has it's own buffer that is stored in the keys attachment. That way different client wont write into the same buffer. Notice that I dont' clear the inputBuffer. The data read is appended to the current content. Here is the code for onBytesRead().


/**
* Adds any packets that are available to the input queue.
*/
void onBytesRead() {
byte packetData[] = null;
while ((packetData = readPacket()) != null) {
synchronized (packetsIn) {
packetsIn.addLast(packetData);
}
if (serverListener != null) {
serverListener.receivedPacket(idx);
}
}
}


/**
* Reads a whole packet content from the inputBuffer.
* @return the packet content or null if the packet is not available.
*/
byte[] readPacket() {
int pos = inputBuffer.position();
if (pos < 2) {
// header not available
return null;
}

int dataLength = inputBuffer.getShort(0);
if (pos < (2+dataLength)) {
// content not available
return null;
}

byte data[] = new byte[dataLength];
inputBuffer.position(2);
inputBuffer.get(data, 0, dataLength);

// move the next packet so that it starts at the beginning of the buffer
inputBuffer.position(0);
int bytesToCopy = pos - dataLength - 2;
for (int i=0; i<bytesToCopy; i++) {
inputBuffer.put(inputBuffer.get(2+dataLength+i));
}

return data;
}



The important stuff is in readPacket(). First it checks if the header is available. If it is it checks if the whole packet is available. If it is the packet is read into a byte array and later returned. The inputBuffer is moved so that the next packet always starts at position 0.

Thank you! I'm starting to understand this now. One thing though (I'm sure I'll have more questions later)... what does this line do:

Connection connection = (Connection) key.attachment();

I'm guessing Connection is your own custom class; it would be helpful if I could see it.

Thanks!

Ok then. It's an inner class of the server:


/**
* A connection to a client.
*/
public class Connection {

/** The index of this connection inside PacketServer.connections array. */
public final int idx;

/** An unique id. No connection will have the same id ever */
public final int uniqueId;

/** The SocketChannel */
public SocketChannel socketChannel;

/** The selection key */
private SelectionKey key;

/** Buffer used to read data */
private ByteBuffer inputBuffer = ByteBuffer.allocate(MAX_PACKET_SIZE);

/** Buffer used to write data */
private ByteBuffer outputBuffer = ByteBuffer.allocate(MAX_PACKET_SIZE);

/** A queue of packets read from client */
private LinkedList packetsIn = new LinkedList();

/** A queue of packets waiting to be sent to client */
private LinkedList packetsOut = new LinkedList();


/**
* Creates a Connection that is connected using the specified
* ServerSocketChannel.
* @param socketChannel
*/
Connection(SocketChannel socketChannel, int connectionId) {
this.socketChannel = socketChannel;
this.idx = connectionId;
this.uniqueId = nextUniqueId++;
outputBuffer.limit(0);

if (connections[idx] != null) {
System.out.println("PacketServer.Connection() Connection warning. Creating Connection over a non null element.");
}

connections[idx] = this;

if (serverListener != null) {
serverListener.connectionAdded(idx);
}
}


/**
* Callback invoced when data has been read into inputBuffer. Will wait
* until a complete packet is in the buffer.
*/
void onBytesRead() {
byte packetData[] = null;
while ((packetData = readPacket()) != null) {
synchronized (packetsIn) {
packetsIn.addLast(packetData);
}
if (serverListener != null) {
serverListener.receivedPacket(idx);
}
}
}


/**
* Reads a whole packet content from the inputBuffer.
* @return the packet content or null if the packet is not available.
*/
byte[] readPacket() {
int pos = inputBuffer.position();
if (pos < 2) {
// header not available
return null;
}

int dataLength = inputBuffer.getShort(0);
if (pos < (2+dataLength)) {
// content not available
return null;
}

byte data[] = new byte[dataLength];
inputBuffer.position(2);
inputBuffer.get(data, 0, dataLength);

// move the next packet so that it starts at the beginning of the buffer
inputBuffer.position(0);
int bytesToCopy = pos - dataLength - 2;
for (int i=0; i<bytesToCopy; i++) {
inputBuffer.put(inputBuffer.get(2+dataLength+i));
}

return data;
}


/**
* Sends a packet to the client.
* @param data the data to send.
*/
void sendPacket(byte data[]) {
synchronized (packetsOut) {
packetsOut.addLast(data);
}

synchronized (pendingWrites) {
pendingWrites.addLast(this);
}

selector.wakeup();
}


/**
* write data to the specified channel.
*/
void write(WritableByteChannel wbc) throws IOException {
int bytesWritten = 0;
do {
// copy packet to outputBuffer if outputBuffer is empty
if (outputBuffer.remaining() <= 0 && packetsOut.size() > 0) {
byte packetData[] = (byte[]) packetsOut.removeFirst();
outputBuffer.limit(outputBuffer.capacity());
outputBuffer.rewind();
outputBuffer.putShort((short) packetData.length);
outputBuffer.put(packetData);
outputBuffer.flip();
}

bytesWritten = 0;
if (outputBuffer.remaining() > 0) {
bytesWritten = wbc.write(outputBuffer);
//System.out.println(bytesWritten+" bytes written");
} else {
//System.out.println("WritableByteChannel deregister write");
int newInterestOps = key.interestOps() & (0xffffffff ^ SelectionKey.OP_WRITE);
key = socketChannel.register(selector, newInterestOps, this);
}
} while (bytesWritten > 0);
}
}



Whoa! I thought that was going to help.  :oops:
I guess I'm confused how casting an Attachment to your Connection class gives you what you need. Isn't the attachment just a payload of bytes? Does it contain all of the channel info you need too?

The attachment is just a user defined Object that I connect to the SelectionKey. I attached the object when I register for a read to the SocketChannel.

I finally figured it out!  :oops:
You actually put your buffer and error checking into the object type that you pass as an attachment.... phew, I think I've got things working now (a lot more code than I thought it would be). I have another question though... everything seems to work and to be solid, but every two seconds my sprites get the "jitters." It's slight, but I've tried everything I can think of to remove them. Is there some flag I need to set in the code to make sure the TCP packets are traveling smoothly? I've tried using setTcpNoDelay(true) and setPerformancePreferences(0,2,1) with no change. Any ideas? Or is it still in the way I'm doing package management?