While working on the X11-AWT peers, I noticed that our RepaintManager gets into trouble when it is called from different threads (which is legal, despite the common mantra that Swing is not thread-safe. The RepaintManager is the core component in the not-thread-safe design of Swing and is actually required to be callable from different threads).
I introduced a new field dirtyComponentsWork, that is swapped with
dirtyComponents in the paintDirtyRegions() method. This is then used to
carry out the painting work, while other threads can safely add new
dirty regions in the other field. This way the locking is kept at a
minimum (I got a deadlock before), while staying thread safe.
The mentioned deadlock was caused by one thread holding the tree-lock of
java.awt.Component, which wanted to acquire the lock on dirtyComponents,
and a second thread (the event thread), holding the lock on dirtyRegions
(in paintDirtyRegions() ), and wanting to acquire the tree-lock in
java.awt.Component. This is resolved by this patch.
2006-04-18 Roman Kennke <[EMAIL PROTECTED]>
* javax/swing/RepaintManager.java
(dirtyComponentsWork): New field.
(ComponentComparator): Use dirtyComponentsWork instead of
dirtyComponents.
(RepaintManager): Initialize new field.
(paintDirtyRegions): Swap dirtyComponents with
dirtyComponentsWork
and work on the copy.
/Roman
--
“Improvement makes straight roads, but the crooked roads, without
Improvement, are roads of Genius.” - William Blake
Index: javax/swing/RepaintManager.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/RepaintManager.java,v
retrieving revision 1.29
diff -u -1 -0 -r1.29 RepaintManager.java
--- javax/swing/RepaintManager.java 27 Mar 2006 14:59:02 -0000 1.29
+++ javax/swing/RepaintManager.java 18 Apr 2006 12:47:52 -0000
@@ -153,23 +153,23 @@
* @return a negative integer, if <code>o1</code> is bigger in than
* <code>o2</code>, zero, if both are at the same size and a
* positive integer, if <code>o1</code> is smaller than
* <code>o2</code>
*/
public int compare(Object o1, Object o2)
{
if (o1 instanceof JComponent && o2 instanceof JComponent)
{
JComponent c1 = (JComponent) o1;
- Rectangle d1 = (Rectangle) dirtyComponents.get(c1);
+ Rectangle d1 = (Rectangle) dirtyComponentsWork.get(c1);
JComponent c2 = (JComponent) o2;
- Rectangle d2 = (Rectangle) dirtyComponents.get(c2);
+ Rectangle d2 = (Rectangle) dirtyComponentsWork.get(c2);
return d2.width * d2.height - d1.width * d1.height;
}
throw new ClassCastException("This comparator can only be used with "
+ "JComponents");
}
}
/**
* A table storing the dirty regions of components. The keys of this
* table are components, the values are rectangles. Each component maps
@@ -181,20 +181,26 @@
*
* @see #addDirtyRegion
* @see #getDirtyRegion
* @see #isCompletelyDirty
* @see #markCompletelyClean
* @see #markCompletelyDirty
*/
HashMap dirtyComponents;
/**
+ * The dirtyComponents which is used in paintDiryRegions to avoid unnecessary
+ * locking.
+ */
+ HashMap dirtyComponentsWork;
+
+ /**
* The comparator used for ordered inserting into the repaintOrder list.
*/
private transient Comparator comparator;
/**
* A single, shared instance of the helper class. Any methods which mark
* components as invalid or dirty eventually activate this instance. It
* is added to the event queue if it is not already active, otherwise
* reused.
*
@@ -255,20 +261,21 @@
*/
private Dimension doubleBufferMaximumSize;
/**
* Create a new RepaintManager object.
*/
public RepaintManager()
{
dirtyComponents = new HashMap();
+ dirtyComponentsWork = new HashMap();
invalidComponents = new ArrayList();
repaintWorker = new RepaintWorker();
doubleBufferMaximumSize = new Dimension(2000,2000);
doubleBufferingEnabled = true;
offscreenBuffers = new WeakHashMap();
repaintUnderway = false;
commitRequests = new HashMap();
}
/**
@@ -547,45 +554,50 @@
/**
* Repaint all regions of all components which have been marked dirty in
* the [EMAIL PROTECTED] #dirtyComponents} table.
*/
public void paintDirtyRegions()
{
// Short cicuit if there is nothing to paint.
if (dirtyComponents.size() == 0)
return;
+ // Swap dirtyRegions with dirtyRegionsWork to avoid locking.
synchronized (dirtyComponents)
{
- // We sort the components by their size here. This way we have a good
- // chance that painting the bigger components also paints the smaller
- // components and we don't need to paint them twice.
- ArrayList repaintOrder = new ArrayList(dirtyComponents.size());
- repaintOrder.addAll(dirtyComponents.keySet());
- if (comparator == null)
- comparator = new ComponentComparator();
- Collections.sort(repaintOrder, comparator);
- repaintUnderway = true;
- for (Iterator i = repaintOrder.iterator(); i.hasNext();)
- {
- JComponent comp = (JComponent) i.next();
- // If a component is marked completely clean in the meantime, then skip
- // it.
- Rectangle damaged = (Rectangle) dirtyComponents.get(comp);
- if (damaged == null || damaged.isEmpty())
- continue;
- comp.paintImmediately(damaged);
- dirtyComponents.remove(comp);
- }
- repaintUnderway = false;
- commitRemainingBuffers();
+ HashMap swap = dirtyComponents;
+ dirtyComponents = dirtyComponentsWork;
+ dirtyComponentsWork = swap;
+ }
+
+ ArrayList repaintOrder = new ArrayList(dirtyComponentsWork.size());;
+ // We sort the components by their size here. This way we have a good
+ // chance that painting the bigger components also paints the smaller
+ // components and we don't need to paint them twice.
+ repaintOrder.addAll(dirtyComponentsWork.keySet());
+
+ if (comparator == null)
+ comparator = new ComponentComparator();
+ Collections.sort(repaintOrder, comparator);
+ repaintUnderway = true;
+ for (Iterator i = repaintOrder.iterator(); i.hasNext();)
+ {
+ JComponent comp = (JComponent) i.next();
+ // If a component is marked completely clean in the meantime, then skip
+ // it.
+ Rectangle damaged = (Rectangle) dirtyComponentsWork.remove(comp);
+ if (damaged == null || damaged.isEmpty())
+ continue;
+ comp.paintImmediately(damaged);
}
+ repaintUnderway = false;
+ commitRemainingBuffers();
}
/**
* Get an offscreen buffer for painting a component's image. This image
* may be smaller than the proposed dimensions, depending on the value of
* the [EMAIL PROTECTED] #doubleBufferMaximumSize} property.
*
* @param component The component to return an offscreen buffer for
* @param proposedWidth The proposed width of the offscreen buffer
* @param proposedHeight The proposed height of the offscreen buffer
signature.asc
Description: Dies ist ein digital signierter Nachrichtenteil
