Синхронизация при удалении и объекте за пределами JPanel

Question

Я пытаюсь удалить объекты, которые находятся за пределами JPanel. Однако, когда я делаю это, я получаю эту ошибку

и моя программа вылетает. Мой лектор сказал мне, что это потому, что два Threads обращаются к ArrayList , в котором хранятся мои объекты.

Я сделал для синхронизации функций, но это не сработало.

пуля

public void move(){
        if(y< -height){
            synchronized (this) {
                bullet.remove(this);
            }
        }
        y-=5;
    }

---

Соответствующие классы:

Применение

import javax.swing.*;

public class Application {
    public static String path ="C:\\Users\\jarek\\OneDrive\\NUIG Private\\(2) Semester 2 2019\\Next Generation Technologies II CT255\\Assignment 3\\";
    private Application(){
        JFrame frame = new JFrame("Ihsan The Defender");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        GamePanel gamePanel= new GamePanel();
        frame.add(gamePanel);
        frame.pack();
        frame.setLocationRelativeTo(null);
        frame.setResizable(false);
        frame.setVisible(true);
        new Thread(gamePanel).start();
    }

    public static void main (String args[]){
        new Application();
    }
}

GamePanel

import javax.swing.*;
import java.awt.*;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import java.security.Key;
import java.util.ArrayList;

public class GamePanel extends JPanel implements Runnable, KeyListener{
    private String path = Application.path;
    Image gameOverImg = new ImageIcon(path+"//images//gameover1.png").getImage();
    private Ihsan ihsan;
    private ArrayList <David> david = new ArrayList<>();
    private int enemies=5;
    private boolean pause=false;
    private boolean gameOver=false;

    GamePanel(){
        ihsan = new Ihsan(this);
        for(int i=0; i<enemies; i++){
            david.add(new David(this));
        }
        setFocusable(true);
        requestFocusInWindow();
        addKeyListener(this);
    }

    @Override
    public void run() {
        while (!pause){
            repaint();
            for(David david:david){
                david.move();
            }
            for(Bullet bullet:Bullet.bullet){
                bullet.move();
            }
            try{Thread.sleep(30);}
            catch (InterruptedException e){}
        }
    }

    public void paint(Graphics g){
        Graphics2D g2d = (Graphics2D) g.create();
        g2d.setColor(Color.GRAY);
        g2d.fillRect(0,0 ,getWidth(), getHeight());

        for(David david : david){
            g2d.drawImage(david.getImg(), david.getX(), david.getY(), null);
        }
        g2d.drawImage(ihsan.getImg(), ihsan.getX(), ihsan.getY(), null);
        for (Bullet bullet:Bullet.bullet){
            g2d.drawImage(bullet.getImg(), bullet.getX(), bullet.getY(), null);
        }

        if(gameOver){
            g2d.drawImage(gameOverImg,0,getHeight()/4,null);
        }
    }

    private static final Dimension DESIRED_SIZE = new Dimension(600,700);
    @Override
    public Dimension getPreferredSize(){
        return DESIRED_SIZE;
    }

    public void setGameOver(boolean gameOver) {
        this.gameOver = gameOver;
    }

    @Override
    public void keyPressed(KeyEvent e) {
        int key=e.getKeyCode();

        if (key==KeyEvent.VK_D || key==KeyEvent.VK_RIGHT){
            ihsan.move(4,0);
            System.out.println("Right Key");
        }

        if (key==KeyEvent.VK_A || key== KeyEvent.VK_LEFT){
            ihsan.move(-4,0);
            System.out.println("Left Key");
        }

        if(key==KeyEvent.VK_SPACE){
            Bullet.bullet.add(new Bullet(this,ihsan.getX()+(ihsan.getWidth()/2), ihsan.getY()));
        }
    }

    @Override
    public void keyTyped(KeyEvent e) { }

    @Override
    public void keyReleased(KeyEvent e) { }

    public boolean getGameOver(){
        return gameOver;
    }
}

пуля

import javax.swing.*;
import java.awt.*;
import java.util.ArrayList;

public class Bullet {
    //Environment
    public static ArrayList<Bullet> bullet = new ArrayList<>();
    private String path = Application.path;
    private GamePanel gp;
    //properties
    private int x,y;
    private int width,height;
    private int yVector;
    private Image image;


    Bullet(GamePanel gp, int x, int y){
        image = new ImageIcon(path+"\\images\\javaicon.png").getImage();
        width=image.getWidth(null);
        height=image.getHeight(null);
        this.gp=gp;
        this.x=x;
        this.y=y;
        yVector=5;
    }

    public void move(){
        if(y< -height){
           bullet.remove(this);
        }
        y-=5;
    }

    public Image getImg(){
        return image;
    }

    public int getX(){
        return x;
    }

    public int getY(){
        return y;
    }
}

Answer 1

Ваша текущая проблема не в синхронизации, а в том, что вы изменяете список маркеров, перебирая его:

// GamePanel.java#run():
    for (Bullet bullet:Bullet.bullet) { //your code is iterating over Bullet.bullet here
        bullet.move(); //you call Bullet#move here
    }

// Bullet.java#move():
    public void move(){
        if(y< -height){
           bullet.remove(this); //this will remove the current bullet from Bullet.bullet
           // ultimately causing the ConcurrrentModificationException in GamePanel.run()
        }
        y-=5;
    }

Синхронизация не поможет, поскольку оба действия происходят в одном потоке.

Чтобы решить эту проблему, метод Bullet.move() должен возвращать логическое значение, указывающее, следует ли его удалить из списка.И GamePanel.run() не должен использовать расширенный цикл for, но итератор (удаление элемента из списка с помощью Iterator.remove() безопасно, если это единственный активный Iterator):

// Bullet.java#move():
    public boolean move(){
        if(y< -height){
           return true; // instruct GamePanel.run() to remove this bullet
        }
        y-=5;
        return false; // keep this bullet
    }

// GamePanel.java#run():
    Iterator<Bullet> it = Bullet.bullet.iterator();
    while (it.hasNext()) {
        Bullet bullet = it.next();
        if (bullet.move()) { // if bullet should be removed
            it.remove(); // remove it from the list
        }
    }

---

Есть и другие проблемы:

• вы звоните #repaint() из своего собственного потока вместо Swing EDT
• перерисовывает итерации по тому же списку Bullet.bullet без синхронизации (что может привести кдо ConcurrentModificationException в пределах GamePanel.paint())

Answer 2

Простое решение проблемы, описанной в Томас Клагер, ответ может быть:

for(Bullet bullet:  new ArrayList(Bullet.bullet) ){ //iterate over a copy
       bullet.move();
}

В качестве альтернативы:

Iterator<Bullet> it = Bullet.bullet.iterator();
while (it.hasNext()) {
    Bullet bullet = it.next();
    bullet.move();
}

без изменения других частей кода.

Answer 3

Блок synchronized должен находиться вокруг каждого фрагмента кода, который обращался к ArrayList или изменял его. Объект в скобках должен быть таким же: это замок.

Например, создайте поле типа Object с именем bulletLock и используйте его в качестве блокировки при каждом доступе к bullet.

Ошибка возникает из-за того, что вы удаляете маркер, когда другой поток находится в цикле for в списке. Поскольку есть параллельная модификация, она не может продолжаться безопасно.

Другим решением было бы сделать копию ArrayList перед циклом for.