Changing owner for ReentrantLock

Question

So I wrote a little program which will move circles around and when they collide they will move opposite direction, however when I'm trying to delay the execution so they won't move around stupidly fast I get java.lang.IllegalMonitorStateException Lock in canvasRender.java, creating an instance:

ReentrantLock renderLock = new ReentrantLock();

Method which will pause execution for a moment, so circles won't move around super fast.

 publlic void delay(){    
        renderLock.unlock();
        try { Thread.sleep(10); } catch (Exception e) {} ;
        renderLock.lock();
    }

then from another class where I create a window and add actionListener

public static void main(String[] args){
//Buttons and other elements
// ...
JButton start = new JButton("Start!");
createAndShowGUI();
}

In createAndShowGUI():

static void createAndShowGUI(){
//adding elements to panels    
start.addActionListener(new ActionListener() {

    @Override
    public void actionPerformed(ActionEvent e) {
        start(); //this will set gameIsRunning variable to true and create models
        while (gameIsRunning) {
            //update(); //which has delay(); at the end of frame drawing
            //but even if just put delay()
            delay(); //still says exception
            start.setEnabled(false); //while game is running button is unavailable
            }
            start.setEnabled(true);
   }
});
}

In this case my lock is owned by the Thread main, but at the time when I click button 'Start!' current is Thread AWT-EventQueue-0, and so the program crashes. How to fix this issue? (or where am I silly?)

score 0 · Accepted Answer · answered Mar 16 '17 at 09:35

The problem is that you're calling renderLock.unlock() from AWT-EventQueue-0 after renderLock.lock() was called by main. The thread AWT-EventQueue-0 isn't allowed to call unlock() it, since it's not the thread that called lock() it in the first place.

You could probably simplify things by dropping ReentrantLock and just using synchronized.

I don't know the design of the rest of your program, but it seems to me that the contents of the while loop belong in a separate thread. You generally don't want to loop in a UI listener method (such as actionPerformed() in ActionListener) because it will freeze up the GUI.

One thing you could do is add an Object to synchronize on:

private static final Object LOCK = new Object()

Then move the game-updating logic to its own thread — something like this:

private static class GameThread extends Thread {
    public GameThread() {
        super("GameThread");
    }

    public void run() {
        synchronized (LOCK) {
            start();
            while (gameIsRunning) {
                update();
                try {
                    // Try to sleep for 10 millis:
                    LOCK.wait(10);
                } catch (InterruptedException ignored) { }
            }
        }
        // Re-enable the button:
        javax.swing.SwingUtilities.invokeLater(() -> start.setEnabled(true));
    }
}

And you can change your ActionListener to simply disable the button and start a GameThread:

start.addActionListener(new ActionListener() {
    @Override
    public void actionPerformed(ActionEvent e) {
        start.setEnabled(false);
        synchronized(LOCK) {
            if(!gameIsRunning) {
                new GameThread().start();
            }
        }
   }
});

Any other code that checks or modifies the state of the game should also be enclosed within a synchronized (LOCK) block. And if update() modifies the GUI as well as the game state, then it probably needs to do so with SwingUtilities.invokeLater().

It might also make things more clear to rename start() to setupGame() and JButton start to JButton startButton.

Holy, that was of much help! I use ReentrantLock to decrease flickering when a lot of objects is being drawn on the screen. Do you think the problem wills till persist if I use synchronizes instead of lock? — Azureus Aeterna, Mar 16 '17 at 12:29
I'm no expert, especially with GUIs, but synchronization/locking is about controlling access to shared resources — probably not related to flickering in your case. I'd take a look at [this answer](https://stackoverflow.com/a/17966278/1232459). Perhaps you should start by updating the UI less frequently. Might be easier to manage if you separate GUI updates from game state updates — e.g. declare a `long lastUIUpdateTime = 0` at the top of `run()`, and only trigger a UI update `if(System.currentTimeMillis() - lastUIUpdateTime > 40)`. (And update `lastUIUpdateTime` when you do so, of course.) — rjb1290, Mar 16 '17 at 15:52

Changing owner for ReentrantLock

1 Answers1