Multithreading deadlocks in Java

Question

I am writing a bot in Java for a game. One thread manages time for the Manager and Worker threads and sends heartbeats for them. Manager collects and interprets messages from the server, and the Worker threads receive commands from the Manager (with detailed information in classes) and acts on this information.

Right now, I'm having trouble with multithreading deadlocks and I'm not sure if there is an alternative.

Here are some methods in my manager class: I call getMessageFromUsername from the GUI thread when the user clicks a button. The following method is called from another thread.

private ArrayList<Message> MessageList = new ArrayList<>();

public synchronized Message getMessageFromUsername(String username) {        
        for( Message msg : MessageList ) {
            if( msg.username.equalsIgnoreCase(username) ) {
                Message m = new Message(msg.num, msg.username, msg.id);                
                return m;
            }
        }

        return null;
    }

My manager thread reads information from the socket and adds information to MessageList in a continuous loop. (Another one, i.e.)

private synchronized void parseMessage() {}
private void main() { while(1) { parseMessage(/*Adds message to MessageList*/); Sleep(); } }

Now, my problem is a pretty noobie issue- because if I want to write to MessageList I have to use synchronized to access it. However, since this happens in a loop and the messages are constantly coming in through the socket it causes a deadlock because it keeps locking my object and this occurs in a loop.

What can I do to solve my problem with deadlocking?

Answer 1

As I said in a comment, you don't have a deadlock, but you starve your thread.

You have two options:

• either you go for some threads with prioritized ordering

• or you go for a concurrency enabled collection. java.util.concurrent has many examples of such collections. But beware - modifying a collection that is foreached is never a good idea. Maybe you just need a kind of a fifo queue? One threads adds a message and another pops it and evaluates?

For example there is ConcurrentLinkedQueue, that provides Add method (to be used by your parseMessage) and isEmpty() (to be used in a while loop of your other thread).

While I'm not a Java expert, I'll point you to this answer: How to use ConcurrentLinkedQueue? It provides some practical advices.

Answer 2

I think the main problem is that you seem to be keeping the lock while waiting for a message to arrive.

You lock via parseMessage() when you start reading a message -- which may not arrive for some time, and the other thread has no chance to do anything until a message actually arrives. And then it might just miss its chance... (wait / notify might solve the latter part of the problem, but I don't think it's needed here).

I'd restructure as follows:

 // just parse and return the message, don't add it to the list
 private Message parseMessage() {} 

 private void main() {
    while(1) {
       Message msg = parseMessage();
       synchronized(messageList) {
         messageList.add(msg);
       }
    } 
 }

Answer 3

If I am understanding your use case correctly then I believe that I'd look into using an implementation of BlockingQueue for your message list. As has already been mentioned there are also concurrent implementations provided such that no external synchronization is required on your part. You should be able to simply poll() or take() from the queue.

Answer 4

I think you should use ArrayBlockingQueue and avoid using loops but replace loops with blocking methods like poll/take/put/offer.

Array based collections perform better and so you should consider first these then Linked node based collections. So ArrayBlockingQueue is better that than ConcurrentLinkedQueue.