CODEWITHSUNDEEP
javamultithreadingcollectionsconcurrency
Monika Michael
Monika Michael

Reputation: 1274

How to synchronize two methods

I have following code for a chat server application in Java - 

public synchronized List<ChatMessage> getMessages(int messageNumber) {
    return messages.subList(messageNumber + 1, messages.size());
}

public synchronized int addMessage(ChatMessage c) {
    messages.add(c);
    return messages.size()-1;
}

I have following test code - 

public static void main(String[] args) {
    final ChatRoom c = new ChatRoom();
    Thread user1 = new Thread(new Runnable() {
        public void run() {
            for(int i=0;i<1000;i++) {
                c.addMessage(new ChatMessage());
                c.getMessages(0);
            }
        }
    });
    Thread user2 = new Thread(new Runnable() {
        public void run() {
            for(int i=0;i<1000;i++) {
                c.addMessage(new ChatMessage());
                c.getMessages(0).size();
            }
        }
    });
    user1.start();
    user2.start();
}

I am getting a ConcurrentModificationException.

How is this possible?

Upvotes: 2

Views: 309

Answers (2)

Peter Lawrey
Peter Lawrey

Reputation: 533520

The simplest thing to do is to create a combined method

public synchronized int addMessageAndGetCount(ChatMessage c) {
    messages.add(c);
    return messages.size();
}

public static void main(String... args) {
    final ChatRoom c = new ChatRoom();
    final Runnable runner = new Runnable() {
        public void run() {
            for(int i = 0; i < 1000; i++) {
                c.addMessageAndGetCount(new ChatMessage());
            }
        }
    };
    new Thread(runner).start();
    new Thread(runner).start();
}

You cannot safely return a list or a subList from a synchronized block. You can return a copy but all you need is the size.

Upvotes: 1

Jon Skeet
Jon Skeet

Reputation: 1500525

How is this possible?

Your getMessages method just returns a view on the original list. It doesn't create a copy of the list. So one thread is using a view on the list while another modifies the list - at that point, you get the exception.

From the docs for List.subList:

The semantics of the list returned by this method become undefined if the backing list (i.e., this list) is structurally modified in any way other than via the returned list. (Structural modifications are those that change the size of this list, or otherwise perturb it in such a fashion that iterations in progress may yield incorrect results.)

It's not clear what you're really trying to achieve here, but fundamentally you can't use subList to magically create a thread-safe list :)

Upvotes: 6

Related Questions