JDK-4988378 : Should synchronize access to Component.{max|min|pref}Size
  • Type: Bug
  • Component: client-libs
  • Sub-Component: java.awt
  • Affected Version: 5.0,7
  • Priority: P4
  • Status: Open
  • Resolution: Unresolved
  • OS: generic
  • CPU: generic
  • Submitted: 2004-02-04
  • Updated: 2024-03-05
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdUnresolved
Related Reports
Relates :  
JDK-4984970 - Java2Demo threw exception in win2003-amd
Relates :  
JDK-4864304 - Improve memory consumption of Swing apps
Relates :  
JDK-6884934 - b4723783 fails due to StringIndexOutOfBoundsException
Description

Name: agR10216			Date: 02/04/2004


The bug 4984970 revealed a synchronization issue with
Component.{max|min|pref}Size and Component.{max|min|pref}SizeSet.
For example, if setMaximumSize() and invalidate() are called on
two different threads at the same time, it's possible for the
component to get into an inconsistent state (the field maxSize
is null, but the field maxSizeSet is true), and a subsequent
call to getMaximumSize() will result in NullPointerException.

###@###.### 2004-02-04
======================================================================
Comments
SUGGESTED FIX Name: agR10216 Date: 02/25/2004 Currently we are weighting "for and against" of using treeLock in Component.setXXXSize(). This might cause a deadlock in an ugly application that worked previously. Maybe it's safer to introduce a new private lock and update Component's sizes under the lock. The latter implies acquiring the private lock after treeLock in all other places where the sizes are updated under treeLock. ###@###.### 2004-02-25 ====================================================================== Name: agR10216 Date: 03/30/2004 The following fix was not approved by the core team (see my discusion of the fix with ###@###.### and ###@###.###'s evaluation presented to the core team in the comments section): ------------------------------------------------------------------------------- *** /net/titan/export/gas/tiger/webrev/src/share/classes/java/awt/Component.java- Wed Feb 25 15:24:39 2004 --- Component.java Wed Feb 25 15:24:36 2004 *************** *** 2144,2162 **** * @since 1.5 */ public void setPreferredSize(Dimension preferredSize) { ! Dimension old; // If the preferred size was set, use it as the old value, otherwise // use null to indicate we didn't previously have a set preferred // size. ! if (prefSizeSet) { ! old = this.prefSize; } ! else { ! old = null; ! } ! this.prefSize = preferredSize; ! prefSizeSet = (preferredSize != null); ! firePropertyChange("preferredSize", old, preferredSize); } --- 2144,2160 ---- * @since 1.5 */ public void setPreferredSize(Dimension preferredSize) { ! Dimension oldDim, newDim; ! newDim = (preferredSize == null ? null : new Dimension(preferredSize)); // If the preferred size was set, use it as the old value, otherwise // use null to indicate we didn't previously have a set preferred // size. ! synchronized (getTreeLock()) { ! oldDim = (prefSizeSet ? this.prefSize : null); ! this.prefSize = newDim; ! this.prefSizeSet = (newDim != null); } ! firePropertyChange("preferredSize", oldDim, newDim); } *************** *** 2216,2234 **** * @since 1.5 */ public void setMinimumSize(Dimension minimumSize) { ! Dimension old; // If the minimum size was set, use it as the old value, otherwise // use null to indicate we didn't previously have a set minimum // size. ! if (minSizeSet) { ! old = this.minSize; } ! else { ! old = null; ! } ! this.minSize = minimumSize; ! minSizeSet = (minimumSize != null); ! firePropertyChange("minimumSize", old, minimumSize); } /** --- 2214,2230 ---- * @since 1.5 */ public void setMinimumSize(Dimension minimumSize) { ! Dimension oldDim, newDim; ! newDim = (minimumSize == null ? null : new Dimension(minimumSize)); // If the minimum size was set, use it as the old value, otherwise // use null to indicate we didn't previously have a set minimum // size. ! synchronized (getTreeLock()) { ! oldDim = (minSizeSet ? this.minSize : null); ! this.minSize = newDim; ! this.minSizeSet = (newDim != null); } ! firePropertyChange("minimumSize", oldDim, newDim); } /** *************** *** 2286,2304 **** * @since 1.5 */ public void setMaximumSize(Dimension maximumSize) { // If the maximum size was set, use it as the old value, otherwise // use null to indicate we didn't previously have a set maximum // size. ! Dimension old; ! if (maxSizeSet) { ! old = this.maxSize; } ! else { ! old = null; ! } ! this.maxSize = maximumSize; ! maxSizeSet = (maximumSize != null); ! firePropertyChange("maximumSize", old, maximumSize); } /** --- 2282,2298 ---- * @since 1.5 */ public void setMaximumSize(Dimension maximumSize) { + Dimension oldDim, newDim; + newDim = (maximumSize == null ? null : new Dimension(maximumSize)); // If the maximum size was set, use it as the old value, otherwise // use null to indicate we didn't previously have a set maximum // size. ! synchronized (getTreeLock()) { ! oldDim = (maxSizeSet ? this.maxSize : null); ! this.maxSize = newDim; ! this.maxSizeSet = (newDim != null); } ! firePropertyChange("maximumSize", oldDim, newDim); } /** *************** *** 2321,2330 **** * @see LayoutManager */ public Dimension getMaximumSize() { ! if (isMaximumSizeSet()) { ! return new Dimension(maxSize); } ! return new Dimension(Short.MAX_VALUE, Short.MAX_VALUE); } /** --- 2315,2325 ---- * @see LayoutManager */ public Dimension getMaximumSize() { ! Dimension dim = maxSize; ! if (dim == null || !(isMaximumSizeSet() || isValid())) { ! dim = new Dimension(Short.MAX_VALUE, Short.MAX_VALUE); } ! return new Dimension(dim); } /** ------------------------------------------------------------------------------- Another fix might be applied [note, a static lock is used since the fix 4864304 (Improve memory consumption of Swing apps) was done in order to reduce memory consumption of a Component]: ------------------------------------------------------------------------------- *** /net/titan/export/gas/tiger/webrev/src/share/classes/java/awt/Component.java- Thu Feb 26 12:07:21 2004 --- Component.java Thu Feb 26 12:07:13 2004 *************** *** 475,480 **** --- 475,485 ---- boolean maxSizeSet; /** + * Code that updates minSize, prefSize or maxSize should hold this lock. + */ + static final Object sizeLock = new Object(); + + /** * The orientation for this component. * @see #getComponentOrientation * @see #setComponentOrientation *************** *** 2144,2162 **** * @since 1.5 */ public void setPreferredSize(Dimension preferredSize) { ! Dimension old; // If the preferred size was set, use it as the old value, otherwise // use null to indicate we didn't previously have a set preferred // size. ! if (prefSizeSet) { ! old = this.prefSize; } ! else { ! old = null; ! } ! this.prefSize = preferredSize; ! prefSizeSet = (preferredSize != null); ! firePropertyChange("preferredSize", old, preferredSize); } --- 2149,2165 ---- * @since 1.5 */ public void setPreferredSize(Dimension preferredSize) { ! Dimension oldDim, newDim; ! newDim = (preferredSize == null ? null : new Dimension(preferredSize)); // If the preferred size was set, use it as the old value, otherwise // use null to indicate we didn't previously have a set preferred // size. ! synchronized (sizeLock) { ! oldDim = (prefSizeSet ? this.prefSize : null); ! this.prefSize = newDim; ! this.prefSizeSet = (newDim != null); } ! firePropertyChange("preferredSize", oldDim, newDim); } *************** *** 2195,2204 **** Dimension dim = prefSize; if (dim == null || !(isPreferredSizeSet() || isValid())) { synchronized (getTreeLock()) { ! prefSize = (peer != null) ? ! peer.preferredSize() : ! getMinimumSize(); ! dim = prefSize; } } return new Dimension(dim); --- 2198,2209 ---- Dimension dim = prefSize; if (dim == null || !(isPreferredSizeSet() || isValid())) { synchronized (getTreeLock()) { ! synchronized (sizeLock) { ! prefSize = (peer != null) ? ! peer.preferredSize() : ! getMinimumSize(); ! dim = prefSize; ! } } } return new Dimension(dim); *************** *** 2216,2234 **** * @since 1.5 */ public void setMinimumSize(Dimension minimumSize) { ! Dimension old; // If the minimum size was set, use it as the old value, otherwise // use null to indicate we didn't previously have a set minimum // size. ! if (minSizeSet) { ! old = this.minSize; } ! else { ! old = null; ! } ! this.minSize = minimumSize; ! minSizeSet = (minimumSize != null); ! firePropertyChange("minimumSize", old, minimumSize); } /** --- 2221,2237 ---- * @since 1.5 */ public void setMinimumSize(Dimension minimumSize) { ! Dimension oldDim, newDim; ! newDim = (minimumSize == null ? null : new Dimension(minimumSize)); // If the minimum size was set, use it as the old value, otherwise // use null to indicate we didn't previously have a set minimum // size. ! synchronized (sizeLock) { ! oldDim = (minSizeSet ? this.minSize : null); ! this.minSize = newDim; ! this.minSizeSet = (newDim != null); } ! firePropertyChange("minimumSize", oldDim, newDim); } /** *************** *** 2264,2273 **** Dimension dim = minSize; if (dim == null || !(isMinimumSizeSet() || isValid())) { synchronized (getTreeLock()) { ! minSize = (peer != null) ? ! peer.minimumSize() : ! size(); ! dim = minSize; } } return new Dimension(dim); --- 2267,2278 ---- Dimension dim = minSize; if (dim == null || !(isMinimumSizeSet() || isValid())) { synchronized (getTreeLock()) { ! synchronized (sizeLock) { ! minSize = (peer != null) ? ! peer.minimumSize() : ! size(); ! dim = minSize; ! } } } return new Dimension(dim); *************** *** 2286,2304 **** * @since 1.5 */ public void setMaximumSize(Dimension maximumSize) { // If the maximum size was set, use it as the old value, otherwise // use null to indicate we didn't previously have a set maximum // size. ! Dimension old; ! if (maxSizeSet) { ! old = this.maxSize; } ! else { ! old = null; ! } ! this.maxSize = maximumSize; ! maxSizeSet = (maximumSize != null); ! firePropertyChange("maximumSize", old, maximumSize); } /** --- 2291,2307 ---- * @since 1.5 */ public void setMaximumSize(Dimension maximumSize) { + Dimension oldDim, newDim; + newDim = (maximumSize == null ? null : new Dimension(maximumSize)); // If the maximum size was set, use it as the old value, otherwise // use null to indicate we didn't previously have a set maximum // size. ! synchronized (sizeLock) { ! oldDim = (maxSizeSet ? this.maxSize : null); ! this.maxSize = newDim; ! this.maxSizeSet = (newDim != null); } ! firePropertyChange("maximumSize", oldDim, newDim); } /** *************** *** 2321,2330 **** * @see LayoutManager */ public Dimension getMaximumSize() { ! if (isMaximumSizeSet()) { ! return new Dimension(maxSize); } ! return new Dimension(Short.MAX_VALUE, Short.MAX_VALUE); } /** --- 2324,2334 ---- * @see LayoutManager */ public Dimension getMaximumSize() { ! Dimension dim = maxSize; ! if (dim == null || !(isMaximumSizeSet() || isValid())) { ! dim = new Dimension(Short.MAX_VALUE, Short.MAX_VALUE); } ! return new Dimension(dim); } /** *************** *** 2406,2428 **** */ public void invalidate() { synchronized (getTreeLock()) { ! /* Nullify cached layout and size information. ! * For efficiency, propagate invalidate() upwards only if ! * some other component hasn't already done so first. ! */ ! valid = false; ! if (!isPreferredSizeSet()) { ! prefSize = null; } - if (!isMinimumSizeSet()) { - minSize = null; - } - if (!isMaximumSizeSet()) { - maxSize = null; - } - if (parent != null && parent.valid) { - parent.invalidate(); - } } } --- 2410,2434 ---- */ public void invalidate() { synchronized (getTreeLock()) { ! synchronized (sizeLock) { ! /* Nullify cached layout and size information. ! * For efficiency, propagate invalidate() upwards only if ! * some other component hasn't already done so first. ! */ ! valid = false; ! if (!isPreferredSizeSet()) { ! prefSize = null; ! } ! if (!isMinimumSizeSet()) { ! minSize = null; ! } ! if (!isMaximumSizeSet()) { ! maxSize = null; ! } ! if (parent != null && parent.valid) { ! parent.invalidate(); ! } } } } *** /net/titan/export/gas/tiger/webrev/src/share/classes/java/awt/Container.java- Thu Feb 26 12:07:23 2004 --- Container.java Thu Feb 26 11:27:41 2004 *************** *** 1551,1560 **** Dimension dim = prefSize; if (dim == null || !(isPreferredSizeSet() || isValid())) { synchronized (getTreeLock()) { ! prefSize = (layoutMgr != null) ? ! layoutMgr.preferredLayoutSize(this) : ! super.preferredSize(); ! dim = prefSize; } } if (dim != null){ --- 1551,1562 ---- Dimension dim = prefSize; if (dim == null || !(isPreferredSizeSet() || isValid())) { synchronized (getTreeLock()) { ! synchronized (sizeLock) { !
17-09-2004
SUGGESTED FIX prefSize = (layoutMgr != null) ? ! layoutMgr.preferredLayoutSize(this) : ! super.preferredSize(); ! dim = prefSize; ! } } } if (dim != null){ *************** *** 1590,1599 **** Dimension dim = minSize; if (dim == null || !(isMinimumSizeSet() || isValid())) { synchronized (getTreeLock()) { ! minSize = (layoutMgr != null) ? ! layoutMgr.minimumLayoutSize(this) : ! super.minimumSize(); ! dim = minSize; } } if (dim != null){ --- 1592,1603 ---- Dimension dim = minSize; if (dim == null || !(isMinimumSizeSet() || isValid())) { synchronized (getTreeLock()) { ! synchronized (sizeLock) { ! minSize = (layoutMgr != null) ? ! layoutMgr.minimumLayoutSize(this) : ! super.minimumSize(); ! dim = minSize; ! } } } if (dim != null){ *************** *** 1615,1627 **** Dimension dim = maxSize; if (dim == null || !(isMaximumSizeSet() || isValid())) { synchronized (getTreeLock()) { ! if (layoutMgr instanceof LayoutManager2) { ! LayoutManager2 lm = (LayoutManager2) layoutMgr; ! maxSize = lm.maximumLayoutSize(this); ! } else { ! maxSize = super.getMaximumSize(); ! } ! dim = maxSize; } } if (dim != null){ --- 1619,1633 ---- Dimension dim = maxSize; if (dim == null || !(isMaximumSizeSet() || isValid())) { synchronized (getTreeLock()) { ! synchronized (sizeLock) { ! if (layoutMgr instanceof LayoutManager2) { ! LayoutManager2 lm = (LayoutManager2) layoutMgr; ! maxSize = lm.maximumLayoutSize(this); ! } else { ! maxSize = super.getMaximumSize(); ! } ! dim = maxSize; ! } } } if (dim != null){ ------------------------------------------------------------------------------- ###@###.### 2004-03-30 ======================================================================
30-03-2004
EVALUATION Name: agR10216 Date: 02/04/2004 A required synchronization should be implemented. ###@###.### 2004-02-04 ====================================================================== ###@###.###, I've assigned you 4971236 which is about this exact issue. Would you please close this (4988378) as a duplicate of 4971236 and address the issue using the earlier bug number? Thank You! ###@###.### 2004-02-17 Name: agR10216 Date: 02/20/2004 As a primary issue is in the demo, the bug 4971236 has been recategorized to demo_2d. Regardless of this fact, committing this bug to tiger-beta2. ###@###.### 2004-02-20 ====================================================================== Name: agR10216 Date: 02/25/2004 Probably one should explain the lack of synchronization in Component.setXXXSize(): these methods were moved from JComponent to Component with the fix for 4864304 (Improve memory consumption of Swing apps). ###@###.### 2004-02-25 ======================================================================
25-02-2004