views:

135

answers:

3

Server

public void run () {

 Socket serversocket = new ServerSocket(port);
 while(true) {
   new Thread(new ServerThread(serverSocket.accept())).start();
 }
}
//serverSocket.close(); etc

ServerThread

public void run() {
 BufferedReader in = new BufferedReader(new InputStreamReader(clientSocket.getInputStream()));

 String input;

 while(true) {
   input = in.readLine();
   new Thread(new RequestThread(clientSocket, input)).start();
 }
}
//close sockets etc in.close() clientSocket.close();

Request Thread

public void run() { 
 //input was passed from constructor
 String output = new SomeProtocol(input);

 if(output == null)
  break;
 //true for auto flush
 PrintWriter out = new PrintWriter(socket.getOutputStream(), true);
 out.println(output);
}//closing out seems to also close the socket, so opted to not to close it, maybe this is the one giving me trouble?

Monitor

public class ServerMonitor implements Runnable{
    private ServerThread server;
    private LoggingClass log = LoggingClass .getInstance();
    private int heartbeat = 0;

    public ServerMonitor(ServerThread server) {
     this.server = server;
    }

    public boolean checkFile() {
     File file = new File("cmd//in//shutdown.txt");
     return file.exists();
    }

    public void run() {

     log.logToFile("Server monitor running");

     while (true) {
      incHeartbeat();
      if (checkFile()) {
       log.logToFile("Shutting down server");
       break;
      }
      writeStatus();
      this.delay(5000);
     }

     log.logToFile("Server monitor stopped");
    }

    public void delay(long delay) {
     try {
      wait(delay);
     } catch (InterruptedException e) {
      log.logToFile("Monitor sleep error");
      e.printStackTrace();
     }
    }

    public void writeStatus() {
     try {
      BufferedWriter out = new BufferedWriter(new FileWriter(
        "sys//status//status.txt"));
      out.write("Start date:" + log.getStartDate());
      out.newLine();
      out.write("Current date:" + log.getTimestamp("yyyy-MMM-dd k:mm:ss"));
      out.newLine();
      out.write("Heartbeat:" + getHeartbeat());
      out.newLine();
      out.write("Cimd in:" + log.getCimdIn());
      out.newLine();
      out.write("Cimd out:" + log.getCimdOut());
      out.newLine();
      out.write("Keep alive:" + log.getCimdKeepAlive());
      out.newLine();
      out.write("HTTP in:" + log.getNumHTTPIn());
      out.newLine();
      out.write("HTTP out:" + log.getNumHTTPOut());
      out.newLine();
      out.close();

     } catch (Exception e) {
      log.logToFile("Write status error");
      e.printStackTrace();
     }
    }

    public int getHeartbeat() {
     return heartbeat;
    }

    public synchronized void incHeartbeat() {
     heartbeat++;
    }
}

This is the rough skeleton of my app. I'm having trouble since sometimes it just stops without any errors. I suspect that it might be because of the sockets but I'm not quite sure so anyone of you guys got any ideas? thanks.


Added my class that monitors the server threads

>How do I know that it doesn't work

Heartbeat doesn't increment anymore

+1  A: 

You may be running out of memory/threads

Do you happen to have some high level exception handling like this:

public static void main( String ... args ) {
    try {
        // lots of code ... 
    } catch( Exception e ) {}
}

If so you have to at least log the errors that you're ignoring.

This is just what comes to my mind for I don't have your source code at hand.

OscarRyz
Yup, I have that. But it doesn't show anything, the thing just stops.
lemon
:-) if you do, remove it just for now, and see what's going on. Or at least make it `catch( Exception e ) { e.printStackTrace(); }` Catching all the exceptions is not a good idea. You should catch what you expect to happen. Or at least log what's going on.
OscarRyz
It doesn't show anything because you're ignoring the exceptions.
OscarRyz
It's set to e.printstacktrace already. And yes I'm logging it too, but just like what I said it just stops, it's still listed in the process handler, but my thread that monitors the server states that it stopped
lemon
When you have system errors ( like OutOfMemoryError ) you don't get much information on the logs, because you are not suppose to handle them. Without seeing much more of the code, it is impossible to determine what's going on. My suggestion is to remove the try/catch altogether and see what happens. If you got exceptions, you may add specific catch sections. Do that, and I think you'll be able to figure out what's going on
OscarRyz
okay I'll replace the try/catch's with function call specific catch's ty.
lemon
Did all of the stuff that you suggested(used a profiler to check the threads, removed default catch) but it still randomly stops. Technically my ServerMonitor class shouldn't stop right? Yet it stops along with the other thread lol.
lemon
A: 

Could you better define 'stops working'? Does it stop accepting connection; does it crash; does it stop doing something in the middle of processing; etc?

One bit of speculation is that I'm not certain what will happen in the reader when a socket is closed. What I would expect to happen is that trying to call read line with a closed socket would cause an exception, which would halt that thread, and/or possibly kill the entire application. Which would generate a stack trace, and other issues.

Basically, in addition to what Oscar said about exception logging in main; you would want logging in any run() method. A try/catch in main would only catch exceptions thrown by the main thread; not those thrown in other threads.

CoderTao
okay, i added how i know why it stops working on my post.
lemon
A: 

It's tough to tell from the pseudo-code you've posted, but if the application is hanging (and not crashing outright) my best guess is that in.readLine() in ServerThread is blocking. If the application is just shutting down, I'd look at socket closures and maybe check isReady() in your BufferedReader.

A general comment: you may be spawning more threads than you need here. It looks like the reading from the client socket and the writing to it happen serially. If this is the case, the ServerThread Runnable (I'm assuming it's a Runnable since you're passing it to the constructor of a Thread) doesn't necessarily need to spawn a RequestThread.

public class ServerRunnable implements Runnable {
    ...

    public void run() {
     BufferedReader in = new BufferedReader(new InputStreamReader(clientSocket.getInputStream()));

      String input;

      while(true)
      {
        input = in.readLine();
        if(input == null)
        {
         //this means the underlying socket closed
         //log something here or return or whatever
        }

        String output = new SomeProtocol(input);

        if(output == null)
         break;
        //true for auto flush
        PrintWriter out = new PrintWriter(socket.getOutputStream(), true);
        out.println(output);
      }
    }
    ...
}
Seth
That's what I did on the previous version, it still stopped tho.
lemon
Oh and think of the server as a dispatcher, each string does it's own thing thats why I made a new thread for each request. Should I revert it back to its previous build?
lemon
Addendum: I doubt that it gets stuck in in.readLine() since the heartbeat of the monitor thread stops along with it(shouldn't happen since it's a different thread altogether). Socket closings: I'm properly closing it I even have my logger logging when the thread stops though when it hangs I don't see it actually logging anything it just stops. (Think of it as someone killed your program while running)
lemon