Errors when trying to send message from client to another client

I have a networking program where i try to get the response from a client and send it over to the server where it should be send to another specific client. On the last step is where the problem occurs. I try to get the client by adding all clients to a hashmap and getting them from there but i get a ConcurrentModificationException from that. Below is my code:

Server:

public class Server {
public static void main(String[] args) {
new Server();
}
ServerSocket serverSocket = null;
static int port = 3339;
static Map<String, Socket> clients = new HashMap<String, Socket>();
private int clientCounter = 0;
public Server() {
try {
serverSocket = new ServerSocket(port);
System.out.println("[SERVER] Server successfully launched on port " + port);
} catch (IOException e) {
System.out.println("[ERROR] Unable to launch server on port " + port);
}
while(true) {
try {
Socket socket = serverSocket.accept();
clientCounter++;
ClientThread client = new ClientThread("Client " + clientCounter, socket);
System.out.println("[SERVER] New client connected: " + client.getClientName() + " (ip:" + socket.getInetAddress() + " port:"
+ socket.getPort() + " localPort:" + socket.getLocalPort() + ")");
clients.put(client.getClientName(), socket);
Thread clientThread = new Thread(client);
clientThread.start();
} catch (IOException e) {
System.out.println("[ERROR] Unable to accept request from client");
}
}
}
}

ClientThread class (used for multithreading)

public class ClientThread implements Runnable{
private String clientName;
private Socket socket;
public ClientThread(String clientName, Socket socket) {
this.socket = socket;
this.clientName = clientName;
}
@Override
public void run() {
/**
* The while(true) loop makes it possible to send multiple responses from client to server back and forth
* without it could only make 1 request because of socket.accept() method
*/
while(true) {
Server.clients.forEach((string, socket) -> {
String message = null;
if(string.equals("Client 1")) {
try {
DataInputStream inputStream1 = new DataInputStream(socket.getInputStream());
DataOutputStream outputStream1 = new DataOutputStream(socket.getOutputStream());
message = inputStream1.readUTF();
} catch (IOException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
}
if(string.equals("Client 2")) {
try {
DataInputStream inputStream2 = new DataInputStream(socket.getInputStream());
DataOutputStream outputStream2 = new DataOutputStream(socket.getOutputStream());
outputStream2.writeUTF(message);
} catch (IOException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
}
});
}
}
public String getClientName() {
return clientName;
}
}

Client:

public class Client {
public static void main(String[] args) {
new Client();
}
public Client()  {
Socket socket = null;
try {
socket = new Socket("127.0.0.1", Server.port);
} catch (UnknownHostException e) {
System.out.println("[ERROR] Error connecting to unknown ip adress");
} catch (IOException e) {
System.out.println("[ERROR] Error connecting to server");
}
try {
while(true) {
System.out.println("You can start typing:");
DataInputStream inputStream = new DataInputStream(socket.getInputStream());
DataOutputStream outputStream = new DataOutputStream(socket.getOutputStream());
Scanner scanner = new Scanner(System.in);
String message;
while(scanner.hasNextLine()) {
message = scanner.nextLine();
if(message.equalsIgnoreCase("quit")) {
socket.close();
break;
}
outputStream.writeUTF(message);
//reading messages from server
String received = inputStream.readUTF();
System.out.println(received);
}
}
} catch (IOException e) {
System.out.println("[ERROR] Unable to get streams from server");
}
}
}

Any help very appriciated.

The short answer... you can get rid of this Exception by using ConcurrentHashMap class. This will fix the problem, I still think you should refactor the code.

The longer answer. See the Java Docs https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html

> The iterators returned by all of this class's "collection view methods" are fail-fast: if the map is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove method, the iterator will throw a ConcurrentModificationException. Thus, in the face of concurrent modification, the iterator fails quickly and cleanly, rather than risking arbitrary, non-deterministic behavior at an undetermined time in the future.

You are starting a thread (i.e. ClientThread) which uses an iterator (i.e. Server.clients.forEach). If this iterator is still active when the next socket is accepted, you will get this exception.

In the server class, you might want to have one socket connection per ClientThread. This would make sense since you are creating a new Thread for each connection, and it would mean you don't need to loop through the connections.