I finally took the plunge and tried to learn NIO. I decided to write an FTP server to make it a real world application. I have attached the code here in hopes that others will find it useful. Also if anyone has any comments, corrections, improvements etc… they would most certainly be welcome. As this is my first attempt, my implementation may be naive and I appreciate anyone pointing out the flaws.
As stated, the point was to learn NIO, so the server meets the minimum standard set out by RFC 959 section 5.1. MINIMUM IMPLEMENTATION, except record structures for file transfer. I also only allows anonymous access. There is also a configuration file to set some variable options like server welcome message and maximum concurrent connections.
Also I released the code under the BSD license so if you want use, go ahead.
I took a peek, though while I’ve written some NIO stuff, I don’t pretend to be an expert.
You allocate 8KB for every read. You might want to allocate one buffer per client. Also you may want to use a direct buffer, but I haven’t personally tested that it is better.
Client#run has @Override but doesn’t, so doesn’t compile for me. Same with Main#run().
In Client#read()… Not sure you need the “channel.isOpen()”? There is no guarantee the channel is still open inside the IF, since it can go bad at any time.
In Client#read() you use “new String(byte[])”, I’m guessing you probably want to explicitly specify a charset?
“messageBufferInUse” looks suspicious. You synchronize on “lock” to set it true and false, but in Client#run you read its value without the lock. It looks like you are trying to avoid the synchronize and sleep instead, but it would be better to just synchronize and your thread will wake up as soon as the monitor is free.
In Client… You copy the new string into a StringBuffer which is thread safe, and you also do the locking yourself. You probably want to use StringBuilder and lock yourself, of StringBuffer and let it do the locking.
Not sure why you start a new Thread in the Client constructor? It seems you were going well with one thread, then fell back to the good old ways when it came to the DataConnection stuff.
With NIO, after connect, generally you should only have OP_READ registered. When you want to write something, you should just try to write it. This almost always succeeds, and avoids the overhead of OP_WRITE. If not all the data could be written, only then should you register both OP_READ and OP_WRITE to write the remainder. When all the data you have is written, you should go back to just OP_READ.
Currently you seem to have OP_WRITE registered all the time. This will introduce unnecessary delays because your data won’t be written until the next select. For FTP this is probably negligible. However, selector.select() in Main#run will always return immediately since it thinks you have data to write, and Client#write will be called repeatedly when there is no data to write. You effectively have a “busy wait”.
I originally started a thread per client so I could deal with commands not being fully read. But now that I think about I could probably do it all in a single thread.
Thanks for all the great points Nate. I will definitely make some changes based on your comments.
Each client gets and instance of Client. Since I allocate 8KB inside the client, that is only 1 buffer per client. It shouldn’t hurt to use direct ByteBuffers so I can make that change.
Both Client and Main implement Runnable and therefore override public void run(). It was Eclipse that put those annotations in. I usually don’t remove them because it doesn’t cause me a problem.
Most of the examples I found used that so I thought it was appropriate.
True.
Yeah, I didn’t quite get that right. I will have to fix it.
I was thinking that since a command may not come through completely in on read I would need to keep checking the message buffer. However I thought of another way that will remove the thread from client and the whole thing can be in one thread.
Main#run calls Client#read every time some data comes in for that client. Client#read allocates 8KB every time it is called.
[quote]I was thinking that since a command may not come through completely in on read I would need to keep checking the message buffer.
[/quote]
Just check it once after each read. If it has enough data to process, do so, otherwise let it be. I think the biggest changes you’ll need to make it single threaded are around the DataConnection stuff. Treat them like you do Client (where Main#run calls methods to read/write/accept as needed) only with different logic (for sending/receiving files rather than processing FTP commands).
Ah, I must have had my compiler set to 1.5. Personally I hate that annotation, as it doesn’t provide any useful information. My IDE already knows the method is implementing or overriding a super class method. It even shows me a little arrow in the gutter I can click to go to the super method. The annotation is just extraneous noise, says I!
But MINA completely hides NIO, so then he wouldn’t get to deal with this horrible, magical API.
