[cp-patches] RFC: deadlock with Componet/CairoGraphics.drawImage()

[Mark Wielaard]

Hi,

I was seeing deadlocks in some cases with ComponentGraphics.drawImage().
ComponentGraphics takes the gdk lock and then calls super.drawImage().
But in some cases when the image isn't a BufferedImage CairoGraphics
would recurse into drawImage() again. And since the gdk lock isn't
reentrant that would cause a deadlock. There was a second problem if the
image was the error image, in that case it wouldn't have a ImageProducer
and later on in CairoGraphics.drawImage() an Exception would be thrown.
In that case the gdk lock would never be released. To prevent against
such situations this patch also moves all operations that have to
operate with the gdk lock held into a try-finally block to make sure the
lock is always released.

2006-06-10  Mark Wielaard  <[EMAIL PROTECTED]>

    * gnu/java/awt/peer/gtk/CairoGraphics2D.java (drawImage):
    Don't recurse, return false if not an BufferedImage and no image
    source available.
    * gnu/java/awt/peer/gtk/ComponentGraphics.java (draw): Add
    end_gdk_drawing() to finally block.
    (fill): Likewise.
    (drawRenderedImage): Likewise.
    (drawImage): Likewise.
    (drawGlyphVector): Likewise.

Does this look sane?

Thanks,

Mark

Index: gnu/java/awt/peer/gtk/CairoGraphics2D.java
===================================================================
RCS file: /cvsroot/classpath/classpath/gnu/java/awt/peer/gtk/CairoGraphics2D.java,v
retrieving revision 1.18
diff -u -r1.18 CairoGraphics2D.java
--- gnu/java/awt/peer/gtk/CairoGraphics2D.java	10 Jun 2006 10:33:17 -0000	1.18
+++ gnu/java/awt/peer/gtk/CairoGraphics2D.java	10 Jun 2006 21:14:49 -0000
@@ -79,6 +79,7 @@
 import java.awt.image.DataBufferInt;
 import java.awt.image.DirectColorModel;
 import java.awt.image.ImageObserver;
+import java.awt.image.ImageProducer;
 import java.awt.image.ImagingOpException;
 import java.awt.image.MultiPixelPackedSampleModel;
 import java.awt.image.Raster;
@@ -1100,11 +1101,14 @@
 				     + xform.toString());
       }
 
-    // Unrecognized image - convert to a BufferedImage and come back.
+    // Unrecognized image - convert to a BufferedImage
     if( !(img instanceof BufferedImage) )
-      return this.drawImage(Toolkit.getDefaultToolkit().
-			    createImage(img.getSource()),
-			    xform, bgcolor, obs);
+      {
+	ImageProducer source = img.getSource();
+	if (source == null)
+	  return false;
+	img = Toolkit.getDefaultToolkit().createImage(source);
+      }
 
     BufferedImage b = (BufferedImage) img;
     DataBuffer db;
Index: gnu/java/awt/peer/gtk/ComponentGraphics.java
===================================================================
RCS file: /cvsroot/classpath/classpath/gnu/java/awt/peer/gtk/ComponentGraphics.java,v
retrieving revision 1.12
diff -u -r1.12 ComponentGraphics.java
--- gnu/java/awt/peer/gtk/ComponentGraphics.java	10 Jun 2006 10:33:18 -0000	1.12
+++ gnu/java/awt/peer/gtk/ComponentGraphics.java	10 Jun 2006 21:14:49 -0000
@@ -181,38 +181,69 @@
   public void draw(Shape s)
   {
     start_gdk_drawing();
-    super.draw(s);
-    end_gdk_drawing();
+    try
+      {
+	super.draw(s);
+      }
+    finally
+      {
+	end_gdk_drawing();
+      }
   }
 
   public void fill(Shape s)
   {
     start_gdk_drawing();
-    super.fill(s);
-    end_gdk_drawing();
+    try
+      {
+	super.fill(s);
+      }
+    finally
+      {
+	end_gdk_drawing();
+      }
   }
 
   public void drawRenderedImage(RenderedImage image, AffineTransform xform)
   {
     start_gdk_drawing();
-    super.drawRenderedImage(image, xform);
-    end_gdk_drawing();
+    try
+      {
+	super.drawRenderedImage(image, xform);
+      }
+    finally
+      {
+	end_gdk_drawing();
+      }
   }
 
   protected boolean drawImage(Image img, AffineTransform xform,
 			      Color bgcolor, ImageObserver obs)
   {
+    boolean rv;
     start_gdk_drawing();
-    boolean rv = super.drawImage(img, xform, bgcolor, obs);
-    end_gdk_drawing();
+    try
+      {
+	rv = super.drawImage(img, xform, bgcolor, obs);
+      }
+    finally
+      {
+	end_gdk_drawing();
+      }
     return rv;
   }
 
   public void drawGlyphVector(GlyphVector gv, float x, float y)
   {
     start_gdk_drawing();
-    super.drawGlyphVector(gv, x, y);
-    end_gdk_drawing();
+    try
+      {
+	super.drawGlyphVector(gv, x, y);
+      }
+    finally
+      {
+	end_gdk_drawing();
+      }
   }
   
   public boolean drawImage(Image img, int x, int y, ImageObserver observer)