How to stop one thread from modifying an array which is being used by another thread?

Question

I have a java program which is basically a game. It has a class named 'World'. The "World" class has a method 'levelChanger()', and another method 'makeColorArray()'.

public class World {

    private BufferedImage map, map1, map2, map3;
    private Color[][] colorArray;

    public World(int scrWd, int scrHi) {
        try {
            map1 = ImageIO.read(new File("map1.png"));
            map2 = ImageIO.read(new File("map2.png"));
            map3 = ImageIO.read(new File("map3.png"));
        } catch (IOException e) {
            e.printStackTrace();
        }
        map = map1;

        makeColorArray();
    }

    private void makeColorArray() {
        colorArray = new Color[mapHi][mapWd]; // resetting the color-array
        for(int i = 0; i < mapHi; i++) {
            for(int j = 0; j < mapWd; j++) {
                colorArray[i][j] = new Color(map.getRGB(j, i));
            }
        }
    }

    //color-array used by paint to paint the world
    public void paint(Graphics2D g2d, float camX, float camY) {
        for(int i = 0; i < mapHi; i++) {
            for(int j = 0; j < mapWd; j++) {
                if(colorArray[i][j].getRed() == 38 && colorArray[i][j].getGreen() == 127 && colorArray[i][j].getBlue() == 0) {
                    //draw Image 1
                }
                else if(colorArray[i][j].getRed() == 255 && colorArray[i][j].getGreen() == 0 && colorArray[i][j].getBlue() == 0) {
                    //draw Image 2
                }
            }
        }
    }

    public void levelChanger(Player player, Enemies enemies) {
        if(player.getRec().intersects(checkPoint[0])) {
            map = map2;
            //calls the color-array maker
            makeColorArray();           
        }
        else if(player.getRec().intersects(checkPoint[1])) {
            map = map3;
            makeColorArray();
        }
    }

    public void update(Player player, Enemies enemies) {
        levelChanger(player, enemies);
    }   
}

The makeColorArray() method makes a 2d array of type 'Color'. This array stores Color-objects from a PNG image. This array is used by the paint() method of the JPanel to paint the world on the screen.

The levelChanger() method is used to change the levels (stages) of the game when certain codition is true. It is the method which calls the makeColorArray() method to re-make the color-array while changing game-level.

The problem is that I have a game-loop which runs on a Thread. Now, the painting of swing components like JPanel is done on a different background thread by java. When the game-level is being changed, the color-array object is re-made. Now, like I previously said, the color-array object is used by the paint() method to paint the world on the screen. Sometimes, the color-array object is still being used by the background Thread (not finished painting) when, according to game logic, color-array object is re-made and its members set to null. This is causing a null-pointer exception, only sometimes. Clearly a race condition.

I want to know how can i stop my game thread from resetting the color-array until the background swing thread has finished painting.

I suggest looking into the java concurrency library: http://docs.oracle.com/javase/tutorial/essential/concurrency/newlocks.html — dckuehn, Dec 19 '14 at 21:21
The best solution is to not use multiple threads. Replace main loop with `javax.swing.Timer` or use `SwingUtilities.invokeAndWait`. — Piotr Praszmo, Dec 19 '14 at 21:22
@Banthar that is a simple solution for the problem but it's not the best for all cases. If the update code is something that takes a long time to process then moving that to the EDT would stop real UI events from being processed in a responsive and timely manner. — NESPowerGlove, Dec 19 '14 at 21:31

score 1 · Answer 1 · answered Dec 19 '14 at 21:22

One way with minimal changes is to synchronize access to the color array.

I personally would have the shared data abstracted out to an individual class which is completely thread-safe, that way you wouldn't have to make sure two separate parts of the code base both have to know what to synchronize on (it seems like your class here does more than just handle the map at first glance, perhaps it is such a class that I am describing though).

private void makeColorArray() {
    Color[][] colorArrayTemp = new Color[mapHi][mapWd]; // resetting the color-array
    for(int i = 0; i < mapHi; i++) {
        for(int j = 0; j < mapWd; j++) {
            colorArrayTemp [i][j] = new Color(map.getRGB(j, i));
        }
    }
    synchronized(colorArray)
    {
         colorArray = colorArrayTemp;
    }
}

//color-array used by paint to paint the world
public void paint(Graphics2D g2d, float camX, float camY) {
    synchronized(colorArray)
    {
        for(int i = 0; i < mapHi; i++) {
            for(int j = 0; j < mapWd; j++) {
                if(colorArray[i][j].getRed() == 38 && colorArray[i][j].getGreen() == 127 && colorArray[i][j].getBlue() == 0) {
                    //draw Image 1
                }
                else if(colorArray[i][j].getRed() == 255 && colorArray[i][j].getGreen() == 0 && colorArray[i][j].getBlue() == 0) {
                    //draw Image 2
                }
            }
        }

    }
}

score 1 · Answer 2 · edited May 23 '17 at 12:33

If you only want one thread modifying colorArray at once, make it synchronized.

The purpose of synchonization, is that it requires a thread to get the lock on the object. Any other thread that attempts to access that object while its locked will be blocked (it will wait).

See this post: Java: how to synchronize array accesses and what are the limitations on what goes in a synchronized condition

Hovercraft Full Of Eels · Accepted Answer · 2014-12-19T21:51:17.080

Suggestions:

Use a Model-View-Control design for your program, or use one of the many similar variants.
Use a Swing Timer to drive your GUI game loop, but use real time slices, not the timer's delay, in your model to determine length of time between loop steps.
The model should run in the GUI Swing event thread.
But its long-running tasks should be run in background threads using a SwingWorker.
This is key: Don't update the model's data, the data used by the JPanel to draw, until the background thread has completed working on it. A PropertyChangeListener and SwingPropertyChangeSupport can be useful for this.
Be sure that the JPanel draws in its paintComponent(...) method, not its paint(...) method, and that you call the super method.
Best if you make the background image a BufferedImage, and have the JPanel draw that in its paintComponent(...) method for efficiency.
Be sure that all Swing GUI code, except perhaps repaint() calls, are called on the Swing event thread.
And yes, definitely read the Concurrency in Swing tutorial.

How to stop one thread from modifying an array which is being used by another thread?

3 Answers3