JOptionPane.ShowOptionDialog not blocking properly

[Warren Dew]

I'm having a problem with Swing not quite behaving as I think it should, and
any help from any Swing gurus out there would be appreciated.
here's excerpts from my code:

    public synchronized void actionPerformed(ActionEvent event)
    {
        if (event.getSource() == Competitor_entry_field)
        {
    
    [a couple levels of function calls omitted]
        
            int item_selected 
                = JOptionPane.showOptionDialog
                (   this, 
                    question_string, 
                    "Seed mark?", 
                    JOptionPane.DEFAULT_OPTION, 
                    JOptionPane.QUESTION_MESSAGE, 
                    null, 
                    choices, 
                    "All dances"
                );
    ...
        }
    }
    ...
    JTextField Competitor_entry_field;

So here's my problem: if the first time I fill out the text field Competitor_entry_field I hit <return> twice in quick succession, two copies of the dialog box come up. I'd think this shouldn't happen, because (a) JOptionPane.showOptionDialog() is supposed to show a modal dialog box, so the second return shouldn't be processed until the first dialog box is dismissed, and (b) JOptionPane.showOptionDialog() is supposed to block, so the same thread can't create a second dialog until the first returns, and another thread can't get to this code because my function is synchronized and still has the lock.
What am I missing here?

[Nathan Pruett]

I don't think you're missing anything... it's just not supposed to do that. What version of Java are you using, what platform, etc. I'm running a quick, thrown together example of your code (below, on Windows 2K and JDK 1.4.2) and everything is happening as it should, with no extra synchronization, etc. needed. If I press Enter repeatedly really fast all that happens is that the dialog pops up and then goes away (as the second Enter chooses the default button on the dialog). Try the example out also and see if you still have the problem of multiple dialogs.

import java.awt.FlowLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
 
import javax.swing.BoxLayout;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JOptionPane;
import javax.swing.JPanel;
import javax.swing.JTextField;
 
public class TextPop extends JFrame implements ActionListener {
    
    private JTextField[] tf;
    
    public TextPop() {
        super( "TextPop" );
        getContentPane().setLayout( new BoxLayout( getContentPane(), BoxLayout.Y_AXIS ) );
        tf = new JTextField[ 5 ];
        JLabel[] l = new JLabel[ tf.length ];
        for ( int i = 0; i < tf.length; i++ ) {
            JPanel p = new JPanel();
            p.setLayout( new FlowLayout( FlowLayout.LEFT ) );
            l[i] = new JLabel( "Field " + (i+1) + ": " );
            p.add( l[i] );
            tf[i] = new JTextField( 13 );
            tf[i].addActionListener( this );
            p.add( tf[i] );
            getContentPane().add( p );
        }
    }
    
    public void actionPerformed( ActionEvent evt ) {
        if ( evt.getSource() == tf[0] ) {
            int item_selected = JOptionPane.showOptionDialog( 
                this, 
                "Bow to your partner?", 
                "Seed mark?", 
                JOptionPane.DEFAULT_OPTION, 
                JOptionPane.QUESTION_MESSAGE, 
                null, 
                new String[] { "All dances", "Some dances", "Only the lambada", "No holds barred dance deathmatch" }, 
                "All dances"
            );
        }
    }
    
    public static void main( String[] args ) {
        JFrame f = new TextPop();
        f.setDefaultCloseOperation( JFrame.EXIT_ON_CLOSE );
        f.setSize( 300, 300 );
        f.setLocationRelativeTo( null );
        f.setVisible( true );
    }
}

[Warren Dew]

Thanks for the discussion. I'm using Java 1.3.1 on Mac OS 10.2.6 (1.4.1 is available but I haven't upgraded yet); my compiler is CodeWarrior Pro 7, though I don't think that should matter.
I tried your example, and I was not able to reproduce the problem with it. However, running your example, the dialog comes up much faster; I'm not certain the second <return> gets to the JVM before it's bringing the dialog up. In my code, the first time (only) that the dialog comes up, there's almost a second wait before it actually appears - plenty of time to get a couple (or a half dozen) <return>s in. I'm assuming the longer wait is due to its being a bigger program, though that might be a bad assumption to make, come to think of it.
Maybe I'll keep tinkering with the two and see if I can figure out where the change in behavior occurs. In the meantime, if anyone has any ideas or knows about any Swing idiosyncracies in 1.3.1 or on OS 10.2, I'd appreciate hearing about them.
Warren

[Nathan Pruett]

I modified my actionPerformed() method to put a 1 second wait in, and I saw the problem you mentioned. If I hit enter a lot, the dialog would pop up multiple times. The reason it takes your dialog longer to come up isn't because it's a bigger app, but because the code in the section you marked "[a couple levels of function calls omitted]" is taking a long time to run, and is possibly blocking on IO, threads, etc. The "quick fix" is to disable your textfield at the start of the actionPerformed() and enable it again at the end. This ensures that it will ignore any further input before it is done with the method. If it's actually taking around a second to pop up the dialog, you should also do something to run the processing in another thread. You don't want to block the event thread for that long... repaints can't get done, and other events can't get processed while your actionPerformed() method is processing, and it makes your GUI look "slow" to the user.

Quick-fix code: (new actionPerformed() method):

    public void actionPerformed( ActionEvent evt ) {
        ((Component)evt.getSource()).setEnabled( false );
        if ( evt.getSource() == tf[0] ) {
            try {
                Thread.sleep( 1000 );
            } catch ( InterruptedException x ) {
                x.printStackTrace();
            }
            int item_selected = JOptionPane.showOptionDialog( 
                this, 
                "Bow to your partner?", 
                "Seed mark?", 
                JOptionPane.DEFAULT_OPTION, 
                JOptionPane.QUESTION_MESSAGE, 
                null, 
                new String[] { 
                        "All dances", 
                        "Some dances", 
                        "Only the lambada", 
                        "No holds barred dance deathmatch" }, 
                "All dances"
            );
        }
        ((Component)evt.getSource()).setEnabled( true );
    }

[Warren Dew]

Thanks - disabling the field for the duration is a good idea; I'll do that. The one second wait only happens the first time the dialog comes up, so I'm pretty sure there's some expensive GUI initialization there (the Mac Aqua look and feel has a lot of expensive GUI effects).
Out of curiousity, if you make your method synchronized, what happens? It still seems to me that synchronization ought to keep the function from being reentered (I only have one instance of the object).
Warren
[ March 09, 2004: Message edited by: Warren Dew ]

[Nathan Pruett]

I think the problem is that if a synchronized method blocks, it gives up the lock and another thread waiting on the method is able to run it. I'm not sure exactly how the event thread does it (custom thread implementation?, thread pool?, Sun secret native implementation?, ), but in any case, you wouldn't want the event thread to *actually* block in this case. That would mean the event thread would block on the JOptionPane method, and the JOptionPane wouldn't be able to even paint itself due to the event thread being blocked.
Check out the following program... it lets you experiment with method synchronization and threading. The InvokeLater and InvokeAndWait options call the methods of the same name in the SwingUtilities class. The runnables passed into these methods actually run on the event thread.

import java.awt.FlowLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.lang.reflect.InvocationTargetException;
 
import javax.swing.BorderFactory;
import javax.swing.ButtonGroup;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JOptionPane;
import javax.swing.JPanel;
import javax.swing.JRadioButton;
import javax.swing.SwingUtilities;
  
public class SynchronizedPopup extends JFrame {
    
    private MyThread[] threads;
    private JRadioButton[] threading;
    private JRadioButton[] method;
    private JButton b;
    
    public SynchronizedPopup() {
        super( "TextPop" );
        getContentPane().setLayout( 
            new FlowLayout( FlowLayout.CENTER ) 
        );
        
        JPanel tp = new JPanel();
        ButtonGroup tbg = new ButtonGroup();
        tp.setBorder( 
            BorderFactory.createTitledBorder( "Threading" ) );
        threading = new JRadioButton[3];
        threading[0] = new JRadioButton( "Normal" );
        threading[1] = new JRadioButton( "InvokeLater" );
        threading[2] = new JRadioButton( "InvokeAndWait" );
        for ( int i = 0; i < threading.length; i++ ) {
            tbg.add( threading[i] );
            tp.add( threading[i] );
        }
        threading[0].setSelected( true );
        
        JPanel mp = new JPanel();
        ButtonGroup mbg = new ButtonGroup();
        mp.setBorder( 
            BorderFactory.createTitledBorder( "Method" ) );
        method = new JRadioButton[2];
        method[0] = new JRadioButton( "Normal" );
        method[1] = new JRadioButton( "Synchronized" );
        for ( int i = 0; i < method.length; i++ ) {
            mbg.add( method[i] );
            mp.add( method[i] );
        }
        method[0].setSelected( true );
        
        b = new JButton( "GO" );
        b.addActionListener( new ActionListener() {
            public void actionPerformed( ActionEvent e ) {
                for ( int i = 0; i < threads.length; i++ ) {
                    threads[i].go();
                }
            }
        } );
        
        getContentPane().add( tp );
        getContentPane().add( mp );
        getContentPane().add( b );
        
        threads = new MyThread[4];
        for ( int i = 0; i < threads.length; i++ ) {
            threads[i] = new MyThread() {
                public void run() {
                    while( true ) {
                        if ( go ) {
                            final boolean synch = method[1].isSelected();
                            if ( threading[0].isSelected() ) {
                                launchPopup( synch );
                            } else if ( threading[1].isSelected() ) {
                                SwingUtilities.invokeLater( new Runnable() {
                                    public void run() {
                                        launchPopup( synch );
                                    }
                                } );
                            } else if ( threading[2].isSelected() ) {
                                try {
                                    SwingUtilities.invokeAndWait( new Runnable() {
                                        public void run() {
                                            launchPopup( synch );
                                        }
                                    } );
                                }
                                catch (InterruptedException x) {
                                    x.printStackTrace();
                                }
                                catch (InvocationTargetException x) {
                                    x.printStackTrace();
                                }
                            }
                            go = false;
                        }
                        else {
                            try {
                                Thread.sleep( 500 );
                            } catch (InterruptedException x) {
                                x.printStackTrace();
                            }
                        }
                    }
                }
                
                public void launchPopup( boolean synch ) {
                    if ( synch ) {
                      synchMethod();
                    } else {
                       normalMethod();
                    }
                } 
            };
            threads[i].setName( "InnerThread" + i );
            threads[i].start();
        }
    }
    
    public synchronized void synchMethod() {
        System.out.println( Thread.currentThread().toString() + 
            " started synchMethod." );
        int item_selected = JOptionPane.showOptionDialog( 
            this, 
            "Bow to your partner?", 
            "Seed mark?", 
            JOptionPane.DEFAULT_OPTION, 
            JOptionPane.QUESTION_MESSAGE, 
            null, 
            new String[] { 
                "All dances", 
                "Some dances", 
                "Only the lambada", 
                "No holds barred dance deathmatch" 
            }, 
            "All dances"
        );
        System.out.println( Thread.currentThread().toString() + 
            " returned from synchMethod popup." );
    }
    
    public void normalMethod() {
        System.out.println( Thread.currentThread().toString() + 
            " started normalMethod." );
        int item_selected = JOptionPane.showOptionDialog( 
            this, 
            "Bow to your partner?", 
            "Seed mark?", 
            JOptionPane.DEFAULT_OPTION, 
            JOptionPane.QUESTION_MESSAGE, 
            null, 
            new String[] { 
                "All dances", 
                "Some dances", 
                "Only the lambada", 
                "No holds barred dance deathmatch" 
            }, 
            "All dances"
        );
        System.out.println( Thread.currentThread().toString() + 
            " returned from normalMethod popup." );
    }
    
    private static class MyThread extends Thread {
        protected boolean go = false;
        
        public void go() {
            this.go = true;
        }
    }
    
    public static void main( String[] args ) {
        JFrame f = new SynchronizedPopup();
        f.setDefaultCloseOperation( JFrame.EXIT_ON_CLOSE );
        f.setSize( 600, 200 );
        f.setLocationRelativeTo( null );
        f.setVisible( true );
    }
}