1

My program works fine and many users can connect and send commands to the server. But when a user spams the server with commands the server blocks out all other clients and the server doesn't receive messages from clients other than the one that spammed. Why is this?

TCPAccept Connections


    package game.server;

import java.io.IOException;
import java.net.Socket;

public class TCPAcceptConnections implements Runnable
{
    public static Socket clientSocket = null;;
    int clientID = -1;

    public void run()
    {
        while(Main.TCP)
        {
            try
            {
                clientSocket = TCPServer.serverSocket.accept();
                System.out.println("Client Connected.");
                clientID++;

                new TCPClientManager(clientSocket, clientID).run();
            } 
            catch (IOException e)
            {
                System.out.println("Couldn't create client socket.");
                System.exit(-1);
            }
        }
    }
}

TCPClientManager:


    package game.server;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.net.Socket;

public class TCPClientManager implements Runnable
{
    Socket client;

    int clientID;

    static PrintWriter out;
    static BufferedReader in;
    String inputLine, outputLine;

    boolean destroy = false;

    public TCPClientManager(Socket cs, int id)
    {
        try
        {
            client = cs;
            clientID = id;
            out = new PrintWriter(client.getOutputStream(), true);
            in = new BufferedReader(new InputStreamReader(client.getInputStream()));
        } catch(IOException e)
        {
            e.printStackTrace();
        }
    }

    public void run()
    {
        System.out.println("Created TCPManager for client.");
        String command;

        while(!destroy)
        {
            try
            {
                if((command = in.readLine()) != null) //If received something
                {
                    System.out.println("Commad received: " + command);
                        System.out.println(" " + Commands.proccessCommand(command));
                    System.out.println("Command proccessed");
                }
                else
                {
                    client.close();
                    destroy = true;
                }
            } catch (IOException e)
            {
                try
                {
                    client.close();
                } catch (IOException e1)
                {
                    e1.printStackTrace();
                    destroy = true;
                }
                System.out.println("Client lost connection.");
                destroy = true;
            }
        }
        System.out.println("TCPManager for client destroyed.");
    }
}

Commands:


package game.server;

public class Commands
{
    public static String proccessCommand(String command)
    {
        if(command.equalsIgnoreCase("cp"))
        {
            System.out.println("Creating player...");
                System.out.println("    Retrieved client");
            return "Player Created";
        }
        else
        {
            return "Unkown command: " + command;
        }
    }
}
1
  • 1
    For reference, you shouldn't be extending Thread unless you're changing how threading works. In cases like yours, where you just have a run method you want the thread to run, implementing Runnable and saying new Thread(new TCPClientManager(clientSocket, clientID)).start(); gets you identical functionality without hinting that threads work differently. Commented Oct 28, 2012 at 1:01

1 Answer 1

1

If you get an unknown command, you should log it and close the connection.

But you have a more severe problem. You aren't stopping the client handler when it reads null. So once a client disconnects the read will spin futilely forever. If readLine() returns null you must close the socket and exit the loop. If you get any IOException you must also close the socket.

Sign up to request clarification or add additional context in comments.

4 Comments

Well this is intended to be a game server, but I don't want them to get disconnected when they enter an invalid command. And how can i constantly check if there is a command being received if i just stop it if it equals null?
@BleedObsidian: If readLine returns null, that means the client has closed its end of the connection. You won't be getting any more commands on that connection. But because you don't exit the loop, you'll just be busy reading null over and over.
OK, I have updated my code above. But now if there is a client already connected it will not pick-up another?
@BleedObsidian Start a new thread with the Runnable, instead of just running it.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.