Making a multithread ftp server Socket problem

Hi , this will be a long one...

Im making a ftp server, briefley like this..
class server
listen on serversocket....
on new connection accept make instance to Client handler.
Each connect client has it's own handler instance witch is a Thread...

in the client handler run method..
while connected...
Read of ftp commands (implemented following the ftp standard)

every thing works good, lots of clients can connect.
Problem accours when a client wants to download / upload large files.

when it get the RETR ftp command it start a instance thread to a class upload. Here i open a new socket and sets it to the conected client adress and transfer port, sets transfermode to 'I'. Inside the read / write loop it just hangs up, i just can't see why
i've been working with this problem for some time week now

Here is a part of the upload class extending thread

Code:

---


class Upload
 private Socket socket;
 private Client  client;
 private String  filename;
 private int      port;

public Upload(Client client, String file, int fport){
 this.port  = fport;
 this.client = client;
 this.filename = file;

try{
InetSocketAddress SocketAddress = new  InetSocketAddress(client.getSocket().getInetAddress(), port));
socket = new Socket(client.getSocket().getInetAddress(), port);
socket.setSoTimeout(2000);
}catch(Exception e){System.out.println("Error");}


public void Run
  if sendmode = aschii then..... this one works just fine
 else send image / binary

// Binary send

 int  buffersize = 4096*2;
 byte buff[]      = new byte[buffersize];
 int  bytesread  = 0;

 readdata  = new BufferedInputStream (new    FileInputStream(filename));
 writedata = new BufferedOutputStream(new DataOutputStream(socket.getOutputStream()));

 socket.setSendBufferSize(buffersize);
   
 while((bytesread = readdata.read(buff,0,buffersize)) >=0){
    writedata .write(buff,0, bytesread);
    writedata .flush();
  }

close sockets and streams....exit thread

---

I've found out that it sends all types of files, as long as they are under ~32Kb of size. With bigger files it just hangs up. I have been debugging the filenames, ports and adresses seem to match. I think the fault is in my othner client class i will take a look in that. But if someone can se a error here please notify me.

Any suggestions ??
Best regards to ya all, thanks for any help.

This is kinda a stab in the dark, but Buffered things, it seems to me at least, seem to be better suited for character data. You might want to use an unbuffered reader such as ByteArrayInputStream() instead. It should work exactly the same way as what you have now. It might be something weird becase java characters are 2-byte instead of 1 and if your file were an odd number of bytes, it might be waiting for another one or something.
Like I said, I am not really sure about this, but I hope this helps. I am working with network stuff too right now, so if you get this to work, I would love to know.

Thx.. i will try it out and repport the result. I just found out that the the binary sending like i did works for small files, i can send every type of file as long as it don't fill upp the buffer. It's strange i worked togheter with two other persons a year ago and we programmed a FTPClient like leechftp supporting allmost unlimited amount of dl / ul threads and this server is constructed the same way.

ugh.. that is ridiculously hard to read! java naming conventions advise that you do NOT use a capital metter to start your variable names.. that is a convention that is reserved for class names, and its the best way for experienced programmers (like me) to tell whether what i'm looking it is a class or a variable.. please amend your code so that it falls within naming conventions, and use the [code] and [/code] tags at the start and end repspectively, to make it appear in preformatted courier font.. then i'll tell you why it doesnt work :)

Sorry for posting my thread so loousy with bad names, varibles and so. I've edited it hope it helps.

thanx to all...:)

i've re-coded your class to make it more generic:

Code:

---

import java.io.*;

public class StreamConnection implements Runnable {
  protected InputStream in;
  protected OutputStream out;

  protected int bufferSize = 8192;
 
  //method name changed
  public upload(InputStream i, OutputStream o){
    in = i;
    out = o;
  }

  //method names must also start with a lowercase letter
  public void run(){
    try {
      //no point selected ascii or binary; just tx everything as binary..

     
      //do not associate the [] with the variable name - [] specifies an array
      //of type, so put it with the type (suppose you want to cast an Object[]
      //into a String[], you write: (String[])myObjectArray;
      //so lets keep the same convention
      byte[] buf      = new byte[buffersize];
     
      //move to inside loop
      //int  bytesRead  = 0;

      //these are not necessary
      //readdata  = new BufferedInputStream (new    FileInputStream(filename));
      //writedata = new BufferedOutputStream(new DataOutputStream(socket.getOutputStream()));

      //this is not necessary. it is/should be set to the MTU of the system anyway
      //socket.setSendBufferSize(buffersize);

      //it is cleaner to use a for loop than a while
      //in.read(byte[]) is the same as in.read(byte[],0,byte[].length)
      for(int bytesRead = in.read(buf); bytesRead>0; bytesRead=in.read(buf)){
        out.write(buf,0, bytesread);
        out.flush();
      }
      //closing these streams will close the socket from whence they came
      in.close();
      out.close();
    } catch(Exception e){ e.printStackTrace(); }
  }
}

---

now, this little component should be used like this:

another class should establish a connection to a file using a FileInputStream. DO not use bufferedinputstream, as this class (StreamConnection) already does buffering with an 8192 byte buffer. adding more buffers will cause things to stop working (as you have found)

the other class should also get the outputstream from the socket, and pass it as the out argument to the constructor..

then, when you want the transfer to proceed you can make a new Thread(), and pass in this new Stream Connection (at the time that you make the new Thread) and then call start() on the new thread:

Code:

---

new Thread(
    new StreamConnection(
        new FileInputStream("c:\blah.txt"),
        socket.getOutputStream()
    ) //streamconn
).start(); //thread

---

the advantage of making this file generic is that it can be used to receive a file too, just state that the IN is the socket.getInputStream() and that the out is a new FileOutputStream(...) and start a new thread of it.. any bytes that appear at the socket will be written to the file..

Thx a lot m8, great awnser there is a few thingies for me to learn there :)

eventually you may want to make the program more functional:

to implement ABORting of a transfer, you should add an extra boolean into your loop condition:

boolean shouldAbort = false;
..
bytesRead>0 && !shouldAbort

then a method of abort() that sets this boolean to true..

not that you will have to make a new SAtreamCOnnection from the control thread, but keep a reference to it,.. if you do:

new Thread(new StreamConnection(...))

then you have not kept a reference.. to keep a reference:

StreamConnection sc = new StreamConn(...)
new Thread(sc).start();


then the control thread can issue sc.abort() if the client requests an abort.


-

to implement resume, conenct the file stream to the file, and skip() N bytes BEFORE you give the stream to the StreamConnection..