CyclicBarrier in listener freezes the program

[Jerry Ye]

Hi,
I'm trying to write a small program in which I have a panel with many buttons. I have to make my main function wait for the user's clicking a button, so I used a CyclicBarrier in the main function and the actionListener of the JButton.
However, it seems to me that if a JButton is pressed, if there's no other program calling CyclicBarrier,  the whole GUI freezes, and even the JFrame can't be closed. How has this happened? Thanks.

With a piece of code:

package test;
import java.awt.*;
import java.awt.event.*;
import java.util.concurrent.BrokenBarrierException;
import java.util.concurrent.CyclicBarrier;

import javax.swing.*;
public class JButtonTest {
	static CyclicBarrier barrier = new CyclicBarrier(2);
	public static void main(String[] args) {
		JFrame frame = new JFrame();
		JPanel panel = new JPanel();
		JButton button1 = new JButton();
		
		button1.addActionListener(new ActionListener(){
			@Override
			public void actionPerformed(ActionEvent e){
				try {
					barrier.await();
					System.out.println("Executed");
				} catch (InterruptedException | BrokenBarrierException e1) {
					// TODO Auto-generated catch block
					e1.printStackTrace();
				}
			}
		});
		panel.add(button1);
		frame.add(panel);
		frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
		frame.setSize(100, 100);
		frame.setVisible(true);
		//test();
	}
	public static void test(){
		try {
			barrier.await();
		} catch (InterruptedException | BrokenBarrierException e) {
			// TODO Auto-generated catch block
			e.printStackTrace();
		}
	}
}

PS. My original code is much more complex. I'm trying to simplify it.

[Henry Wong]

Jerry Ye wrote:
However, it seems to me that if a JButton is pressed, if there's no other program calling CyclicBarrier,  the whole GUI freezes, and even the JFrame can't be closed. How has this happened?

Swing has a single thread to handle all events -- ie. the event dispatching thread. This thread is used to do all the drawing, mouse handling, menus, etc. along with the processing of the callbacks. So, if the EDT is stuck in your callback, simply waiting for the CyclicBarrier to release it, then nothing in Swing can happen -- as there isn't an available thread to do anything.

Henry

[Jerry Ye]

Henry Wong wrote:
Swing has a single thread to handle all events -- ie. the event dispatching thread.
Henry

If that's the case, how should I design my program to wait for the user's input?

[Henry Wong]

Jerry Ye wrote:
If that's the case, how should I design my program to wait for the user's input?

Why do you actually need to wait for input in a callback? Isn't that what the GUI is for? When a button is pressed, when text is entered, when the mouse moves, etc. etc. etc., you get an event for it. The GUI's job is to wait for input and report it to you as event callbacks.

Henry

[Jerry Ye]

Henry Wong wrote:
Why do you actually need to wait for input in a callback? Isn't that what the GUI is for? When a button is pressed, when text is entered, when the mouse moves, etc. etc. etc., you get an event for it. The GUI's job is to wait for input and report it to you as event callbacks.

Henry

Well, that's true. But I'm writing a game for two users, and I have to wait for the input of one user to get to the other user and to repaint the GUI. How should I design my classes then? My original idea was I write a thread, tells one GUI to get input, wait for the response and then gets to the other one.

[Henry Wong]

How are these two GUIs connected? Are they connected by a network? Or is it just one GUI (with 2 JFrames) handling two different users?

In the case of the network, you have to use a thread that is not the EDT. For example, you can create a task (that is to be run) for/by an executor. This task can do the external event, like wait for the other user's response. Upon completion, Swing also provides a way to register a callback to be called by the EDT, if needed. Or if the tasks after completion doesn't need to access Swing components, you can stay with the executor.

Henry

[Jerry Ye]

Henry Wong wrote:
How are these two GUIs connected? Are they connected by a network? Or is it just one GUI (with 2 JFrames) handling two different users?

In the case of the network, you have to use a thread that is not the EDT. For example, you can create a task (that is to be run) for/by an executor. This task can do the external event, like wait for the other user's response. Upon completion, Swing also provides a way to register a callback to be called by the EDT, if needed. Or if the tasks after completion doesn't need to access Swing components, you can stay with the executor.

Henry

It's just one GUI with 2 JFrame/JPanel handling two different users. I just don't get why my program didn't run as it supposed to do. I'm trying to repeat my original problem with a simplified program. This might take me several days.

Thanks a lot.

[Campbell Ritchie]

Removed from Beginning forum as too difficult a question.
Simply: don't do anything strange with the Thread which Swing components run on, otherwise it will hang. Remember that GUI frameworks are usually not thread‑safe and can only be run from the one thread. If you start making things wait or anything like that on the Swing thread (called EDT=Event Dispatch Thread), you will interfere with painting of the GUI and it may stop responding

[Jerry Ye]

Campbell Ritchie wrote:Don't do anything strange with the Thread which Swing components run on, otherwise it will hang. Remember that GUI frameworks are usually not thread‑safe and can only be run from the one thread. If you start making things wait or anything like that on the Swing thread (called EDT=Event Dispatch Thread), you will interfere with painting of the GUI and it may stop responding

Well, I just used a CyclicBarrier with 2 parties and it supposed to continue immediately when the user clicks the button as the other thread has already been waiting. However, strangely, when I was debugging, I found that barrier.getNumberWaiting returned 0 and I have no idea how it has happened. Another thread has indeed called await and is waiting, but I still got 0 for barrier.getNumberWaiting().

[Jerry Ye]

Here's part of the source code. If you need the entire code, I can send by email
main class:

import javax.swing.*;
import java.awt.*;
import java.util.ResourceBundle;
import java.util.Locale;
import java.util.concurrent.*;
public class SeaBattle extends JFrame implements Runnable{
	private Board board1;
	private Board board2;
	private static ResourceBundle bundle;
	private ExecutorService service = Executors.newSingleThreadExecutor();
	static {
		//Locale.setDefault(Languages.chooseLanguage(Languages.getAvailableLanguages("main")));
		bundle = ResourceBundle.getBundle("language.main");
	}
	
	public SeaBattle(){
		super(bundle.getString("sea_battle"));
		board1 = new Board(bundle.getString("user_1"));
		board2 = new Board(bundle.getString("user_2"));
		this.add(board1);
		this.add(board2);
		
		this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
		this.pack();
		this.setVisible(true);
		try{
			service.submit(this::run);
		}
		finally{
			service.shutdown();
		}
	}
	public void run(){
		while(true){
			board1.setVisible(true);
			board1.getChoice(); // Wait for user choice here
			if(board1.isFinished()) break;
			board1.setVisible(false);
			board2.setVisible(true);
			board2.getChoice();
			if(board2.isFinished()) break;
			board2.setVisible(false);
		}
	}
	public static void main(String args[]){
		SeaBattle game = new SeaBattle();
	}

}

Board class:

// imports
public class Board extends JPanel{
	/**
	 * The size of the board.
	 */
	protected static final int SIZE = 10;
	/**
	 * The gap between components.
	 */
	protected static final int GAP = 5;
	/**
	 * The NPoints that constitute this board.
	 */
	private NPoint[][] cases = new NPoint [SIZE][SIZE];
	/**
	 * The ships of the player.
	 */
	private Ship[] ships = new Ship[5];
	/**
	 * A CyclicBarrier to wait for the choice of user.
	 */
	private CyclicBarrier choiceBarrier = new CyclicBarrier(2, () -> {System.out.println("Reached");});
	public Board(String playerName){
		this.setLayout(new GridLayout(SIZE, SIZE, GAP, GAP));
		for(int i = 0; i < SIZE; i++){
			for(int j = 0; j < SIZE; j++){
				cases[i][j] = new NPoint(i, j);
				cases[i][j].addActionListener(new ActionListener(){
					@Override
					public void actionPerformed(ActionEvent e){
						try{
							System.out.println("Executed");
							// Here called the await
							Board.this.choiceBarrier.await();
						} catch(InterruptedException | BrokenBarrierException exception){
							System.err.println(exception);
						}
					}
				});
				this.add(cases[i][j]);
			}
		}
		this.setPreferredSize(new Dimension(SIZE * (NPoint.SIZE + GAP), SIZE * (NPoint.SIZE + GAP)));
		// initialize ships
		class ShipPlacer extends JPanel implements MouseListener, MouseMotionListener{
			// an inner class for initialization
		}
		ShipPlacer placer = new ShipPlacer();
		placer.getChoice(); // getChoice of the inner class, not the outer class
		this.setPreferredSize(new Dimension(Board.SIZE * NPoint.SIZE + (Board.SIZE - 1) * Board.GAP, Board.SIZE * NPoint.SIZE + (Board.SIZE - 1) * Board.GAP));
	}
	void getChoice(){
		try{
			System.out.println("Executed");
			// Another await called
			this.choiceBarrier.await();
		} catch(InterruptedException | BrokenBarrierException exception){
			System.err.println(exception);
		}
	}
    // something else
}

[Tony Docherty]

You're creating two instances of the Board class and so you have two different CyclicBarriers each waiting for 2 threads. When the first one's await() method is called by the EDT (Event Dispatch Thread) it is locked and therefore, as others have already explained, the whole GUI freezes and there will be no more GUI events and hence the await() method is never called again but even if it was your code still wouldn't work because it would be called on a different instance of CyclicBarrier.

[Jerry Ye]

Tony Docherty wrote:You're creating two instances of the Board class and so you have two different CyclicBarriers each waiting for 2 threads.

I printed their hashcode. You are right. They aren't waiting for the same object. But how could this be possible? I've used this keyword and they are registered to two different instances?

[Henry Wong]

Jerry Ye wrote:
I printed their hashcode. You are right. They aren't waiting for the same object. But how could this be possible? I've used this keyword and they are registered to two different instances?

Not sure what the this keyword has anything to do with this... Basically, you instantiated 2 Board instances, and each instance of Board instantiated a cyclic barrier. So, why wouldn't there be two barriers?

Of course, isn't this all moot? You only have one EDT for all of swing, so, you don't have two threads for the barrier to release, even if you fix the barrier instantiation issue.

Henry

[Jerry Ye]

Henry Wong wrote:
Not sure what the this keyword has anything to do with this...

Well, I mean, if I initiated with the barrier of the JPanel that contains it, how can it call the await of the other JPanel? After all, they are two different instances, right?

Of course, isn't this all moot? You only have one EDT for all of swing, so, you don't have two threads for the barrier to release, even if you fix the barrier instantiation issue.

Indeed, I has only one EDT, but I have also another thread I started myself in background, so it should work. And it indeed worked in another class that I've written. I think the problem is indeed the barrier instantiation issue.

[Tony Docherty]

Both Henry and I have explained why it isn't going to work when you are calling await() from the EDT and even if you are using another thread (which I can't see in the code shown) we have also both explained why your design means there are two different CyclicBarrier objects so it still can't possibly work.

May I suggest you forget about CyclicBarriers for now and answer Henry's much earlier post where he said:

Henry wrote:Why do you actually need to wait for input in a callback? Isn't that what the GUI is for? When a button is pressed, when text is entered, when the mouse moves, etc. etc. etc., you get an event for it. The GUI's job is to wait for input and report it to you as event callbacks.

[Henry Wong]

Jerry Ye wrote:
Well, I mean, if I initiated with the barrier of the JPanel that contains it, how can it call the await of the other JPanel? After all, they are two different instances, right?

It is just a data structure. You need to setup an access path that gets to the same instance. Yes, it may be two references, but it goes to the same instance.

How about creating the instance prior, and passing it as part of the construction process?

Henry

[Henry Wong]

Jerry Ye wrote:
Indeed, I has only one EDT, but I have also another thread I started myself in background, so it should work. And it indeed worked in another class that I've written. I think the problem is indeed the barrier instantiation issue.

Not really. Remember that the barrier works in a threaded environment. If the background thread gets to the barrier first, then you are golden. However, if the EDT gets to the barrier first, then the GUI is frozen until the background thread gets there.  

It may be a small pause. And your clients may be okay with it... until of course, they're not...

Henry

[Henry Wong]

Tony Docherty wrote:
May I suggest you forget about CyclicBarriers for now and answer Henry's much earlier post

Agreed. Keep in mind that Swing, like all GUIs are event based. And arguably, when working with Swing, it is probably a good idea to code in events, instead of threads.

Henry

[Jerry Ye]

Tony Docherty wrote:
May I suggest you forget about CyclicBarriers for now and answer Henry's much earlier post where he said:

Henry wrote:Why do you actually need to wait for input in a callback? Isn't that what the GUI is for? When a button is pressed, when text is entered, when the mouse moves, etc. etc. etc., you get an event for it. The GUI's job is to wait for input and report it to you as event callbacks.

Sorry for not responding.
Well, what I want to do is to tell the GUI make some changes, get the user's choice and do something else. I know I can get an event for it, but the point is that my main thread has to wait for it, doesn't it? So I'm just trying to figure out how I can wait for the response. I think it's kind of like the JDialog, when all we need is to wait for user click the "OK" button. But even if we have added the actionlistener to the button, we will have to make the main thread wait for it, right? And we have to make it notify the main thread, right? I just thought that CyclicBarrier may be easier to realize this.
And what I mean the other thread is the main thread. So the two threads the CyclicBarrier is waiting is EDT and the main thread.

How about creating the instance prior, and passing it as part of the construction process?

Well, I used static to solve it. I admit what I've written is a poor design, but I just want to know why it didn't work in the way I expected.

[Campbell Ritchie]

I presume you know about Swing threading(←Java™ Tutorials section). That link tells you about background threads. You can monitor the background thre‍ad and show its progress with progress indicators.

[Jerry Ye]

Campbell Ritchie wrote:I presume you know about Swing threading(←Java™ Tutorials section). That link tells you about background threads. You can monitor the background thre‍ad and show its progress with progress indicators.

OK, thanks a lot.

[Henry Wong]

Jerry Ye wrote:

How about creating the instance prior, and passing it as part of the construction process?

Well, I used static to solve it. I admit what I've written is a poor design, but I just want to know why it didn't work in the way I expected.

Like I said, it is just data structures. You can setup the access path to two different references that point to the same instance. Or you can setup the access path, to get to the same reference, which of course, gets to the same instance (as you did via the use of static variables -- assuming, since we never seen the code). Either option is fine, as long as you get to the same instance.

In your example above, you setup the access path to two different barrier references that point to their own (ie. different) instances -- initialized when the board instances were initialized. Of course, this is not what you wanted.

Regardless, this part of the discussion is likely moot. The Swing threading tools, mentioned in the tutorial provided by Campbell, isn't supposed to use this type of thread communication... and perhaps, we should have mentioned the tutorial first...

Henry