Java UDP - server accepting packets on multiple ports

Question

I have quite a complex question here, I hope understanding my code won't be such a problem.

I'm writing a program based on UDP and TCP communication in Java. Server is listening on several UDP ports (1. number and quantity of ports is given by a user in program parameters; 2. A Thread is created for each port) for a packets from Clients (there can be more than one trying to send a packet at a time to Server). Each packet contains Clients id, the message and UDP port number of the Client who send this packet. Server receives the packet, puts the message in a HashMap (Client id is the key, sent messages are stored in a List of Strings). On each packet received, Servers checks the List of Strings whether the messages sent from specified Client are matching a password. If the messages are in correct order, Server sends a generated port number for TCP communication with the Client who has sent the correct password, opens ServerSockets, they perform a simple communication and the Client closes.

Now, a Client should be able to send his messages to various ports. For example, Server is listening on ports 2000 and 3000. A Client should be able to send 2 messages to port 2000 and another two messages on port 3000. However, the Server seems to be be receiving messages only on the first opened port.

If a Client sends all his messages on one port, it all works fine.

Here is the Server class:

import java.io.BufferedReader;
import java.io.DataInputStream;
import java.io.DataOutputStream;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.net.DatagramPacket;
import java.net.DatagramSocket;
import java.net.InetAddress;
import java.net.ServerSocket;
import java.net.Socket;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Set;

public class Server {

    static int portsOpenedQuantity;
    static HashMap<String, List<String>> packetsReceived = new HashMap<>();
    static List<Integer> portsTCP = new ArrayList<>();

    public static void main(String[] args) {
        portsOpenedQuantity = args.length;

        List<String> listOfPorts = new ArrayList<>();
        for(int i = 0; i < portsOpenedQuantity; ++i)
            if(!listOfPorts.contains(args[i]) && (Integer.parseInt(args[i]) > 1024))
                listOfPorts.add(args[i]);       

        for(int i = 0; i < listOfPorts.size(); ++i) {
            final int j = i;
            System.out.println("SERVER listening on port: " + listOfPorts.get(j));

            Thread listeningPort = new Thread(new Runnable() {
                @Override
                public void run() {
                    synchronized (packetsReceived){
                        try {
                            byte[] packetReceived = new byte[256];
                            DatagramSocket ds = new DatagramSocket(Integer.parseInt(listOfPorts.get(j)));
                            DatagramPacket dp = new DatagramPacket(packetReceived, 256);
                            while(true){
                                ds.receive(dp);
                                List<String> sequence = new ArrayList<>();
                                System.out.println("SERVER received a packet");
                                String msgReceived = new String(dp.getData(), 0, dp.getLength());
                                String[] separatedMsg = msgReceived.split(" ");
                                int portUDPNumber = Integer.parseInt(separatedMsg[2]);
                                System.out.println("Id: " + separatedMsg[0]);
                                System.out.println("Value: " + separatedMsg[1]);
                                System.out.println("Port UDP: " + separatedMsg[2]);
                                if(packetsReceived.containsKey(separatedMsg[0])) {
                                    sequence = packetsReceived.get(separatedMsg[0]);
                                    packetsReceived.remove(separatedMsg[0]);
                                    System.out.println(separatedMsg[1]);
                                    sequence.add(separatedMsg[1]);
                                    System.out.println(sequence);
                                    packetsReceived.put(separatedMsg[0], sequence);
                                } else {
                                    System.out.println(sequence);
                                    sequence.add(separatedMsg[1]);
                                    packetsReceived.put(separatedMsg[0], sequence);
                                }

                                String sequenceResult = "";
                                for(int k = 0; k < sequence.size(); ++k) {
                                    sequenceResult += sequence.get(k);
                                }
                                System.out.println(sequenceResult);

                                if(sequenceResult.equals("!@#$")){
                                    System.out.println("Connecting via TCP...");
                                    int portNumber = (int)((Math.random()*100)+5000);
                                    boolean portAvailable = true;
                                    ServerSocket ss = null;
                                    System.out.println("TCP port number: " + portNumber);
                                    while(portAvailable) {
                                        try{
                                            ss = new ServerSocket(portNumber);
                                            portsTCP.add(portNumber);
                                            portAvailable = false;
                                        } catch(Exception e) {
                                            portAvailable = true;
                                            portNumber++;
                                        }
                                    }
                                    System.out.println("socket number aquired");
                                    String portNr = portNumber+"";
                                    byte[] portNrToSend = portNr.getBytes();
                                    dp = new DatagramPacket(portNrToSend, portNrToSend.length, InetAddress.getByName("localhost"), portUDPNumber);
                                    System.out.println("Datagram created");
                                    ds.send(dp);
                                    System.out.println("Datagram sent");
                                    Socket s = ss.accept();
                                    System.out.println("Port number sent to: " + portUDPNumber);
                                    PrintWriter out = new PrintWriter(s.getOutputStream(), true);
                                    BufferedReader in = new BufferedReader(new InputStreamReader(s.getInputStream()));          
                                    String msgFromClient = in.readLine();
                                    System.out.println("Message from client: " + msgFromClient);
                                    out.println("I received your message");
                                    in.close();
                                    out.close();
                                    s.close();
                                    ss.close();
                                }
                            }
                        } catch (Exception e) {
                            e.printStackTrace();
                        }
                    }
                }
            });
            listeningPort.start();
        }
    }

}

And the Client class:

import java.io.BufferedReader;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.net.DatagramPacket;
import java.net.DatagramSocket;
import java.net.InetAddress;
import java.net.ServerSocket;
import java.net.Socket;
import java.net.SocketException;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.List;

public class Client {

    InetAddress ip;
    String idClient;
    List<Integer> portsUDP = new ArrayList<>();
    String sequence;

    public Client(String[] args) {
        try {
            ip = InetAddress.getByName(args[0]);
            idClient = args[1];
            for(int i = 2; i < args.length; ++i)
                portsUDP.add(Integer.parseInt(args[i]));
            this.sequence = "!@#$"; 

            DatagramSocket ds = new DatagramSocket();
            DatagramPacket dp = null;
            for(int i = 0; i < portsUDP.size(); ++i) {
                byte[] toSend = new byte[256];
                String msgToSend = idClient + " " + sequence.charAt(i) + " " + ds.getLocalPort();
                System.out.println("CLIENT named as: " + idClient + " sends a message: " + sequence.charAt(i) + " " + ds.getLocalPort());
                toSend = msgToSend.getBytes();
                dp = new DatagramPacket(toSend, toSend.length, ip, portsUDP.get(i));
                ds.send(dp);
                System.out.println("CLIENT: " + idClient + " sent a packet");
                toSend = new byte[256];
                msgToSend = "";
            }

            String received;
            byte[] tabReceived = new byte[256];
            dp = new DatagramPacket(tabReceived, tabReceived.length);
            System.out.println("Datagram created");
            ds.receive(dp);
            System.out.println("Datagram received");
            received = new String(dp.getData(), 0, dp.getLength());
            System.out.println("Received TCP port number: " + received);
            int portTCP = Integer.parseInt(received);

            int portNumber = (int)((Math.random()*100)+5000);
            boolean portAvailable = true;
            ServerSocket ss = null;
            while(portAvailable) {
                try{
                    ss = new ServerSocket(portNumber);
                    portAvailable = false;
                } catch(Exception e) {
                    portAvailable = true;
                    portNumber++;
                }
            }
            System.out.println("ServerSocket created");
            Socket s = new Socket(ip, portTCP);
            System.out.println("Socket created");
            PrintWriter out = new PrintWriter(s.getOutputStream(), true);
            BufferedReader in = new BufferedReader(new InputStreamReader(s.getInputStream()));
            out.println("Succes of communication");
            String msgFromServer = in.readLine();
            System.out.println("Message from SERVER: " + msgFromServer);
            in.close();
            out.close();
            s.close();
            ss.close();
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

    public static void main(String[] args) {
        new Client(args);
    }

}

I think what is creating problems here is the synchronization on the HashMap, but when I run it without synchronization, the packets come in a completely random sequence, are not stored in the HashMap properly - it's just much worse.

I'd be grateful for any suggestions and comments.

Answer 1

Your Runnable loops forever after obtaining a lock on a shared object called packetsReceived, so only one of your Threads will actually be able to accomplish anything; the other thread will wait for the lock forever.

You should be able to verify this with a simple thread dump while your sample program is running.

The solution (to the synchronization problem) is to only obtain that lock when you want to actually modify the HashMap. So, remove the synchronized(packetsReceived) from outside the loop and put it around the code that actually performs the containsKey/remove/put calls on the HashMap.