Bug ID: JDK-4177735 JColorChooser does not release 2 threads which are created for each instance

Type: Bug
Component: client-libs
Sub-Component: javax.swing
Affected Version: 1.2.0

Priority: P4
Status: Resolved
Resolution: Fixed
OS: windows_nt
CPU: x86

Submitted: 1998-09-30
Updated: 2013-11-01
Resolved: 2001-07-25

Other
1.4.0 beta2Fixed


Name: rk38400			Date: 09/30/98


Every time a new instance of JColorChooser is created (and added to some container), it creates 2 threads, which are never destroyed.
The number of these threads increases with any new instance !!!

    "Thread-3" (TID:0xd747e0, sys_thread_t:0x9a6e60, Win32ID:0xfb, state:CW) prio=2
        java.lang.Object.wait(Object.java:315)
        com.sun.java.swing.plaf.basic.HueLightnessPatch$1.waitNextColor(BasicHSVChooserPanel.java:549)
        com.sun.java.swing.plaf.basic.HueLightnessPatch$1.computeRow(BasicHSVChooserPanel.java:561)
        com.sun.java.swing.SyntheticImageGenerator.run(SyntheticImage.java:126)
    "Thread-2" (TID:0xd748b0, sys_thread_t:0x9a53f0, Win32ID:0x128, state:CW) prio=2
        java.lang.Object.wait(Object.java:315)
        com.sun.java.swing.plaf.basic.ColorPatch$1.waitNextColor(BasicHSVChooserPanel.java:656)
        com.sun.java.swing.plaf.basic.ColorPatch$1.computeRow(BasicHSVChooserPanel.java:668)
        com.sun.java.swing.SyntheticImageGenerator.run(SyntheticImage.java:126)
(Review ID: 39612)
======================================================================

CONVERTED DATA BugTraq+ Release Management Values COMMIT TO FIX: merlin merlin-beta2 FIXED IN: merlin-beta2 INTEGRATED IN: merlin-beta2

14-06-2004

SUGGESTED FIX *** /tmp/geta4853 Tue Jul 17 17:11:10 2001 --- DefaultHSBChooserPanel.java Tue Jul 17 17:08:41 2001 *************** *** 16,21 **** --- 16,22 ---- import javax.swing.event.*; import javax.swing.border.*; import java.awt.image.*; + import java.beans.*; /** * Implements the default HSB Color chooser *************** *** 561,578 **** protected float[] hsb = new float[3]; protected boolean isDirty = true; - // Timer fields to check to see if the thread should stick around. - private long startWait; - private static final long TIMER_WAIT = 10000; // check every 10 sec. - private static final long THRESHOLD_WAIT = 60000; // let thread die after 60 sec - - // Debug values to speed things up - // private static final long TIMER_WAIT = 1000; - // private static final long THRESHOLD_WAIT = 6000; - protected AbstractHSBImage( int width, int height, float h, float s, float b ) { super( width, height ); setHSB( h, s, b ); } public void setHSB( float h, float s, float b ) { --- 562,572 ---- protected float[] hsb = new float[3]; protected boolean isDirty = true; protected AbstractHSBImage( int width, int height, float h, float s, float b ) { super( width, height ); setHSB( h, s, b ); + DefaultHSBChooserPanel.this. + addPropertyChangeListener(new ThreadStopper()); } public void setHSB( float h, float s, float b ) { *************** *** 611,623 **** public synchronized void nextFrame( int param ) { isDirty = true; - - /* if (!isUseful()) { - // Reset the useful bit if the thread timed out. - setIsUseful(true); - restartThread(); - } */ - notifyAll(); } --- 605,610 ---- *************** *** 652,668 **** */ protected void computeRow( int y, int[] row ) { if ( y == 0 ) { - startWait = System.currentTimeMillis(); - // XXX - debug - // System.out.println("Start wait: " + startWait); synchronized ( this ) { try { ! while ( !isDirty && isUseful()) { ! wait(TIMER_WAIT); ! checkUsefulness(); } } catch ( Exception e ) { ! System.out.println( e ); } isDirty = false; } --- 639,651 ---- */ protected void computeRow( int y, int[] row ) { if ( y == 0 ) { synchronized ( this ) { try { ! while ( !isDirty ) { ! wait(); } } catch ( Exception e ) { ! System.out.println( e ); } isDirty = false; } *************** *** 671,693 **** row[i] = getRGBForLocation( i, y ); } } ! ! /** ! * Fix for 4177735: The thread that gets spawned by the image ! * will stay around for the lifetime of the application. The ! * reference to the thread will be unreachable. This method checks after ! * certain period, the last time that this image was changed and will ! * invalidate it after a threshold period ! */ ! private void checkUsefulness() { ! // XXX - debug ! // System.out.println("Check thread"); ! if (System.currentTimeMillis() - startWait > THRESHOLD_WAIT) { ! synchronized ( this ) { ! // XXX - debug ! // System.out.println("Set thread to not useful..."); ! setIsUseful(false); ! isDirty = true; } } } --- 654,674 ---- row[i] = getRGBForLocation( i, y ); } } ! ! class ThreadStopper implements PropertyChangeListener { ! public void propertyChange(PropertyChangeEvent ev) { ! if ("ancestor".equals(ev.getPropertyName())) { ! if (isShowing()) { ! setIsUseful(true); ! isDirty = true; ! restartProduction(); ! } else { ! setIsUseful(false); ! isDirty = true; ! synchronized (AbstractHSBImage.this) { ! AbstractHSBImage.this.notifyAll(); ! } ! } } } } ###@###.###

11-06-2004

EVALUATION Very, very difficult. The problem is in the AbstractHSBImage which extends SyntheticImage which in implemented for the DefaultHSBChooserPanel. the problem is that the computeRow() method will block the thread created in SyntheticImageGenerator and it will not allow itself to be garbage collected. I partially fixed the problem by adding the removeConsumer() method which will unblock the thread and allow it to terminate. However, this method will not be called when the chooser terminates for the paletteImage and the sliderImage. I've also made the paletteImage and sliderImage field static so that new ones are not created so that the only two threads will be created for the color chooser rather than a proliferation of threads for every instance. This is much better behaviour and at least it won't leak out of control. The best solution is to detect when the DefaultHSBChooserPanel is not in use and unblock the thread (isDirty = true; notifyAll()) so that it completes. mark.davidson@Eng 2000-02-14 Name: pzR10082 Date: 07/09/2001 The image producer thread should be stopped as soon as it is no longer needed. The problem is how to determine when it is. Ideally, we should wait until the HSB chooser panel is garbage collected. It turns out, however, that the thread in question keeps an indirect link to it, so it's never collected as long as the thread runs. It seems we should force the thread to stop at some moment, and be able to restart it later, should this be needed. E.g. we could stop when HSB chooser panel becomes invisible, and restart when it's made visible again. ###@###.### 2001-07-09 ====================================================================== Attached a test case to verify that the threads are being released: import java.awt.*; import java.awt.event.*; import javax.swing.*; import javax.swing.event.*; import javax.swing.colorchooser.*; /** * Instructions: Display the color chooser, switch to the HSB tab and click on the * Hue, Saturation and Brightness radio buttons. When you press the kill button no threads * should be displayed. * * @version %I% %G% * @author Mark Davidson */ public class ThreadTest extends JFrame implements ChangeListener, ActionListener { public JColorChooser chooser; public JDialog dialog; public JPanel panel = new JPanel(); public JButton button; public JButton kill; /** * ctor */ public ThreadTest() { button = new JButton("Chooser"); button.addActionListener(new ActionListener() { public void actionPerformed(ActionEvent evt) { createColorChooser(); } }); kill = new JButton("Kill Chooser"); kill.addActionListener(new ActionListener() { public void actionPerformed(ActionEvent evt) { destroyColorChooser(); } }); panel.setPreferredSize(new Dimension(200, 200)); getContentPane().add(button, BorderLayout.NORTH); getContentPane().add(panel, BorderLayout.CENTER); getContentPane().add(kill, BorderLayout.SOUTH); } private void createColorChooser() { chooser = new JColorChooser(); chooser.getSelectionModel().addChangeListener(this); dialog = JColorChooser.createDialog(this, "Color Test", true, chooser, this, this); chooser.setColor(new Color(204, 0, 204)); dialog.setVisible(true); Color color = chooser.getColor(); panel.setBackground(color); panel.repaint(); System.out.println("Color Selected: " + color); } private void destroyColorChooser() { if (chooser != null) { chooser.getSelectionModel().removeChangeListener(this); chooser = null; } if (dialog != null) { dialog.dispose(); dialog = null; } System.gc(); dumpThreads(); } private void dumpThreads() { ThreadGroup tg = Thread.currentThread().getThreadGroup(); int n = tg.activeCount(); Thread list[] = new Thread[n]; int count = tg.enumerate(list, false); int daemons = 0; Thread t; System.out.println("\n" + n + " Threads in thread group " + tg); System.out.println("Daemon Threads:"); for (int i = 0; i < count; i++) { t = list[i]; String threadName = t.getName(); // Show SyntheticImage threads and kick out the TimerQueue if (t.isDaemon() && !"TimerQueue".equals(threadName) && !"AWT-Windows".equals(threadName)) { daemons++; System.out.println(list[i]); } } if (daemons != 0) { System.out.println("FAILED! Residual Daemons: " + daemons); } } public void stateChanged(ChangeEvent evt) { ColorSelectionModel model = (ColorSelectionModel)evt.getSource(); System.out.println("Color Model Changed. Color: " + model.getSelectedColor()); } public void actionPerformed(ActionEvent evt) { dumpThreads(); } public static void main(String[] args) { ThreadTest test = new ThreadTest(); test.addWindowListener(new WindowAdapter() { public void windowClosing(WindowEvent evt) { System.exit(0); } }); test.pack(); test.setVisible(true); } } mark.davidson@Eng 2001-07-19

19-07-2001

Relates :	JDK-6718965 - Swing color chooser tests should be open source
Relates :	JDK-4515373 - JCK14 2 api/javax_swing/interactive/JColorChooserTests.html#JColorChooser