CODEWITHSUNDEEP
javaconcurrency
MikeW
MikeW

Reputation: 4809

how to do I safely iterate over a collection inside a thread?

I have a java / blazeds app that keeps an array "_games" which is 2 players subscribed to the same blazeds topic. The issue as I see is less than 40% of connection attempts succeed, so far my efforts at debugging the issue have been unsuccessful - it seems to throw silently , and the behaviour is random except that the 1st pair seem to have more luck connecting than subsequent attempts.I have seen a ConcurrentModificationException and what I have read on the topic suggests my code has problems with concurrency.

In this case I'm iterating over a array "_games" inside a thread, I'd like to know how I can do this safely as other clients will be attempting to access this variable at random times.

Update #2:

While debugging the for loop inside the thread, it appears the current item is always the same so the iterator is not advancing and or being reset by a subsequent request so i always = 0 , or if use a for(Game g : _games) , then g is always the 1st item in the array.

thanks!

public class GameService {

private static List<Game> _games;
private static GameServiceThread thread;


public GameService(MessageTemplate template) {
    this.template = template;
}

public Game CreateOrJoinGame(GamePlayer player)
{

    Game currentGame = new Game();
    boolean isFull = false;
    for (Game g: _games)
    {

        ArrayCollection myCollection = g.myCollection;
        currentGame = g;
        if(!g.IsFull())
        {
            myCollection.add(player);

            if(g.IsFull())
            {
                isFull = true;
                currentGame.myCollection = myCollection;
                System.out.print("User Count..." + myCollection.size() + "\n");
            }
            break;
        }

    }
    if(isFull)
    {
        return currentGame;
    }
    else
    {
        Game creator = CreateGame(player);
        return creator;
    }
}


public void start() {
     if (thread == null) {
        if(_games == null)
        {
          _games = new ArrayList<Game>();
        }        
            thread = new GameServiceThread(this.template);
            thread.start();
     }
}
public void AddPlayer(GamePlayer player)
{
    _allPlayers.add(player);
}

//nested static thread class

public static class GameServiceThread extends Thread {

        @Override
    public void run() {

        this.running = true;
        while (this.running) 
        {
            for (Game g: _games)
            {
                 //do work
            }

        }
     }


    private void sendGameUpdate(final Game game) {}

This service is registered in flex-servlet just for info, although I don't think this is my issue.

<bean id="gameFeedService" class="com.acme.services.GameService">
    <constructor-arg ref="defaultMessageTemplate" />
    <flex:remoting-destination />
</bean>

Edit: Added some code for how new games get added. I iterate over _games inside the GameService class and also inside the GameServiceThread. Firstly as new clients send a remote game object to the service, and I iterate inside the same collection to send each game recipient a message. Inside each game is a collection which I use to determine if the Game is full or not - if it is a new Game() is created.

Upvotes: 1

Views: 115

Answers (4)

deronius
deronius

Reputation: 11

You could synchronize on the list itself. 

    @Override
public void run() {

    this.running = true;
    while (this.running) 
    {
        synchronized (_games) {
            for (Game g: _games) {
             //do work
            }
        }
    }
 }

Upvotes: 1

xagyg
xagyg

Reputation: 9711

synchronized(_games) {            
    for (Game g: _games)
    {
        //do work
    }
}

OR 

synchronized(getClass()) {            
    for (Game g: _games)
    {
        //do work
    }
}

will both work.

Depending on what you are doing in sendGameUpdate, you may have to synchornize on the same object there as well.

Upvotes: 0

minus
minus

Reputation: 2786

You can start by using a thread safe List, for example:

List list = Collections.synchronizedList(new ArrayList(...));

Check ArrayList javadoc for further informations.

Upvotes: 0

Victor Zamanian
Victor Zamanian

Reputation: 3180

You might need to synchronize access to the _games list.

Here's a Synchronization tutorial on docs.oracle.com.

Upvotes: 0

Related Questions