[cairo] Huge leak in Cairo/win32

[Hans Breuer]

While playing with gtk/win32's Cairo integration I spotted
a huge leak also visible with cairo_snippets_win32. To see
it simply go through the snippets with key 'n' or 'p' and
watch the memory usage with the task manager.

To plug it apply the attached patch :-)

The leak was caused by missing 'unselect' of the cairo/win32
internal surface->bitmap. Failing to do so lets the following
DeleteObject(bitmap) call fail as well.
Also in case of a self-created dc that needs to be deleted, too.

BTW: cairo-win32.h looks like it got commited with win32
line endings from unix. Getting it back with win32 cvs
gives 0x0d 0x0d 0x0a. Difficult to patch but can this be
fixed as well?

Thanks,
	Hans


-------- Hans "at" Breuer "dot" Org -----------
Tell me what you need, and I'll tell you how to
get along without it.                -- Dilbert
-------------- next part --------------
diff --exclude-from=c:\util\tool\diff.ign -u --recursive from-cvs/cairo/cairo/src/cairo_win32_surface.c my-gtk/cairo/cairo/src/cairo_win32_surface.c
--- from-cvs/cairo/cairo/src/cairo_win32_surface.c	Fri Feb 04 08:52:14 2005
+++ my-gtk/cairo/cairo/src/cairo_win32_surface.c	Sat Feb 12 21:58:46 2005
@@ -98,19 +98,17 @@
 }
 
 static cairo_status_t
-_create_dc_and_bitmap (HDC             original_dc,
-		       cairo_format_t  format,
-		       int             width,
-		       int             height,
-		       HDC            *dc_out,
-		       HBITMAP        *bitmap_out,
-		       char          **bits_out,
-		       int            *rowstride_out)
+_create_dc_and_bitmap (cairo_win32_surface_t *surface,
+		       HDC                    original_dc,
+		       cairo_format_t         format,
+		       int                    width,
+		       int                    height,
+		       char                 **bits_out,
+		       int                   *rowstride_out)
 {
-    HDC dc = NULL;
-    HBITMAP bitmap = NULL;
     cairo_status_t status;
 
+    HBITMAP oldbitmap = NULL;
     BITMAPINFO *bitmap_info = NULL;
     struct {
 	BITMAPINFOHEADER bmiHeader;
@@ -121,6 +119,9 @@
     int num_palette = 0;	/* Quiet GCC */
     int i;
 
+    surface->dc = NULL;
+    surface->bitmap = NULL;
+
     switch (format) {
     case CAIRO_FORMAT_ARGB32:
     case CAIRO_FORMAT_RGB24:
@@ -191,26 +192,24 @@
 	}
     }
 
-    dc = CreateCompatibleDC (original_dc);
-    if (!dc)
+    surface->dc = CreateCompatibleDC (original_dc);
+    if (!surface->dc)
 	goto FAIL;
 
-    bitmap = CreateDIBSection (dc,
-			       bitmap_info,
-			       DIB_RGB_COLORS,
-			       &bits,
-			       NULL, 0);
-    if (!bitmap)
+    surface->bitmap = CreateDIBSection (surface->dc,
+			                bitmap_info,
+			                DIB_RGB_COLORS,
+			                &bits,
+			                NULL, 0);
+    if (!surface->bitmap)
 	goto FAIL;
     
-    if (!SelectObject (dc, bitmap))
+    if (!(oldbitmap = SelectObject (surface->dc, surface->bitmap)))
 	goto FAIL;
 
     if (bitmap_info && num_palette > 2)
 	free (bitmap_info);
 
-    *dc_out = dc;
-    *bitmap_out = bitmap;
     if (bits_out)
 	*bits_out = bits;
 
@@ -231,7 +230,9 @@
 	    break;
 	}
     }
-    
+
+    /* to get back our original bitmap we need to select this into the dc */
+    surface->bitmap = oldbitmap;
     return CAIRO_STATUS_SUCCESS;
 
  FAIL:
@@ -240,12 +241,17 @@
     if (bitmap_info && num_palette > 2)
 	free (bitmap_info);
 
-    if (bitmap)
-	DeleteObject (bitmap);
-    
-    if (dc)
-	DeleteDC (dc);
-    
+    if (surface->bitmap) {
+        if (oldbitmap)
+            surface->bitmap = SelectObject (surface->dc, oldbitmap);
+	DeleteObject (surface->bitmap);
+    }
+    surface->bitmap = NULL;
+
+    if (surface->dc) 
+	DeleteDC (surface->dc);
+    surface->dc = NULL;
+
     return status;
 }
 
@@ -257,8 +263,6 @@
 				    int	            height)
 {
     cairo_win32_surface_t *surface;
-    HDC dc = NULL;
-    HBITMAP bitmap = NULL;
     char *bits;
     int rowstride;
 
@@ -266,9 +270,9 @@
     if (!surface)
 	return NULL;
 
-    if (_create_dc_and_bitmap (original_dc, format,
+    if (_create_dc_and_bitmap (surface, original_dc, format,
 			       width, height,
-			       &dc, &bitmap, &bits, &rowstride) != CAIRO_STATUS_SUCCESS)
+			       &bits, &rowstride) != CAIRO_STATUS_SUCCESS)
 	goto FAIL;
 
     surface->image = cairo_image_surface_create_for_data (bits, format,
@@ -277,8 +281,6 @@
 	goto FAIL;
     
     surface->format = format;
-    surface->dc = dc;
-    surface->bitmap = bitmap;
     
     surface->clip_rect.x = 0;
     surface->clip_rect.y = 0;
@@ -293,10 +295,13 @@
     return (cairo_surface_t *)surface;
 
  FAIL:
-    if (bitmap)
-	DeleteObject (bitmap);
-    if (dc)
-	DeleteDC (dc);
+    /* we have either bot or none at all, but only by _create_dc_and_bitmap() */
+    assert (!surface->bitmap == !surface->dc);
+    if (surface->bitmap && surface->dc) {
+        surface->bitmap = SelectObject (surface->dc, surface->bitmap);
+	DeleteObject (surface->bitmap);
+	DeleteDC (surface->dc);
+    }
     if (surface)
 	free (surface);
     
@@ -350,8 +355,12 @@
     if (surface->saved_clip)
 	DeleteObject (surface->saved_clip);
 
-    if (surface->bitmap)
+    if (surface->bitmap) {
+        surface->bitmap = SelectObject (surface->dc, surface->bitmap);
 	DeleteObject (surface->bitmap);
+        /* having the bitmap does mean we own the dc */
+        DeleteDC (surface->dc);
+    }
 
     free (surface);
 }