[cairo] Fix for PostScript printer crash (1.6.2 plans for today)

Carl Worth cworth at cworth.org
Fri Apr 11 09:20:16 PDT 2008


On Thu, 10 Apr 2008 17:57:48 -0700, Carl Worth wrote:
> [*] Note: Just before this release, a bug has been reported that the
> PostScript output from cairo can crash some printers, (so far the
> following models have been reported as problematic Xerox Workcentre
> 7228 or 7328 and Dell 5100cn). We will implement a workaround as soon
> as we can learn exactly what in cairo's output these printers object
> to, (and we could use help from users that have access to misbehaving
> printers). This bug is being tracked here:
>
> 	Printing some PDFs from evince is crashing our Xerox printer
> 	http://bugs.freedesktop.org/show_bug.cgi?id=15348

And just after the release, Adrian found and fixed the bug here:

	commit c5814d2aa3cb68a13bc9cc8b6a47f660febcad71
	Author: Adrian Johnson <ajohnson at redneon.com>
	Date:   Fri Apr 11 21:42:19 2008 +0930

	    PS: Fix inefficient implementation of Tm/Td operators that was
	    crashing printers

The root cause was actually interesting. Our implementation in
PostScript of the PDF Tm and Td operations would establish a new font
matrix, (with new translation components), every few glyphs. This
would cause the PostScript interpreter to allocate new resources for a
new font object each time, (even though the non-translation components
of the font matrix were unchanged). And apparently, the PostScript
interpreter inside some commercially available printers just can't cope
with font objects being created so frequently.

So Adrian fixed this by being careful to always put 0 values into the
translation components of the PostScript font matrix, and instead use
moveto for positioning. This fix has been tested and verified to fix
problems on both brand-names of printers that had exhibited a problem.

So I'd like to push out an immediate cairo 1.6.2 release with this
fix.

I have also reviewed the other commits that have happened since 1.5.20
and I've chosen one other to include in 1.6.2. A thread-safety fix for
cairo-xlib caught by Chris Wilson:

	commit dc714106e156cb7901e376c0935922446ae9bcdf
	Author: Chris Wilson <chris at chris-wilson.co.uk>
	Date:   Thu Apr 10 14:49:47 2008 +0100

	    [xlib] Add locking around GC cache.

	    The per-screen cached of most-recently freed GCs lacks suitable
	    locking for it to be threadsafe.

I've attached below all the proposed changes from 1.6.0 to 1.6.2 for
review, (of course, I'll still add release notes to NEWS and increment
the version number again).

Hopefully this release can live for a bit longer than one day. :-)

Still having fun,

-Carl

-------------- next part --------------
From cf057c1e8603014033c079189369e91aecac2adf Mon Sep 17 00:00:00 2001
From: Adrian Johnson <ajohnson at redneon.com>
Date: Fri, 11 Apr 2008 21:42:19 +0930
Subject: [PATCH] PS: Fix inefficient implementation of Tm/Td operators that was crashing printers

The Td and Tm operator emulation were setting the font matrix like this:

  /some_font [xx yx xy yy x0 y0] selectfont

where [xx yx xy yy] is the font matrix and [x0 y0] is the position of
the first glyph to be drawn. This seemed to be the easiest way to
emulate the Tm operator since the six arguments to Tm required by PDF
are xx,yx,xy,yy,x0,y0.

Before the switch to pdf-operators the font matrix was set like this:

  /somefont [xx yx xy yy 0 0] selectfont x0 y0 moveto

The selectfont operator is equivalent to calling findfont, makefont,
and setfont. The makefont operator creates a new font dictionary for
specified font that contains the specified font matrix. The
description of the makefont operator in the PostScript Language
Reference Manual states:

  "The interpreter keeps track of font dictionaries recently created
   by makefont. Calling makefont multiple times with the same font and
   matrix will usually return the same font rather than create a new
   one."

So the emulation of Tm and Td was creating a new font dictionary every
time a text string was displayed due to the change in the translation
components of the font matrix. Previously the font dictionary was
re-used as with the translation components of the matrix set to zero,
the font matrix did not change frequently.

Some printers did not handle well the frequent creation a font
dictionary every time a few glyphs were displayed.

Fix this by ensuring the translation components of the font matrix
used in the emulation of Tm and Td operators is always set to
zero. Use moveto instead for the translation components.
(cherry picked from commit c5814d2aa3cb68a13bc9cc8b6a47f660febcad71)
---
 src/cairo-ps-surface.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/cairo-ps-surface.c b/src/cairo-ps-surface.c
index 54f5afe..f6940bf 100644
--- a/src/cairo-ps-surface.c
+++ b/src/cairo-ps-surface.c
@@ -183,10 +183,11 @@ _cairo_ps_surface_emit_header (cairo_ps_surface_t *surface)
 				 "    { show } { -0.001 mul 0 cairo_font_matrix dtransform rmoveto } ifelse\n"
 				 "  } forall\n"
 				 "} bind def\n"
-				 "/Td { matrix translate cairo_font_matrix matrix concatmatrix dup\n"
-				 "   /cairo_font_matrix exch def cairo_font exch selectfont 0 0 moveto } bind def\n"
-				 "/Tm { 6 array astore dup /cairo_font_matrix exch def\n"
-				 "      cairo_font exch selectfont 0 0 moveto } bind def\n"
+				 "/Td { matrix translate cairo_font_matrix matrix concatmatrix aload\n"
+				 "      /cairo_font_matrix exch def 6 2 roll 0 0 6 array astore\n"
+				 "      cairo_font exch selectfont moveto } bind def\n"
+				 "/Tm { 6 copy 6 array astore /cairo_font_matrix exch def 6 2 roll 0 0\n"
+				 "      6 array astore cairo_font exch selectfont moveto } bind def\n"
 				 "/g { setgray } bind def\n"
 				 "/rg { setrgbcolor } bind def\n");
 
-- 
1.5.5.rc0.24.g86c30

-------------- next part --------------
From 9cfd82e87b60c0d65e9cafda026cb9a498874575 Mon Sep 17 00:00:00 2001
From: Chris Wilson <chris at chris-wilson.co.uk>
Date: Thu, 10 Apr 2008 14:49:47 +0100
Subject: [PATCH] [xlib] Add locking around GC cache.

The per-screen cached of most-recently freed GCs lacks suitable locking
for it to be threadsafe.
(cherry picked from commit dc714106e156cb7901e376c0935922446ae9bcdf)
---
 src/cairo-xlib-private.h |    1 +
 src/cairo-xlib-screen.c  |   31 +++++++++++++++++++++----------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/src/cairo-xlib-private.h b/src/cairo-xlib-private.h
index 5bfc2ec..2d9737d 100644
--- a/src/cairo-xlib-private.h
+++ b/src/cairo-xlib-private.h
@@ -79,6 +79,7 @@ typedef struct _cairo_xlib_visual_info {
 struct _cairo_xlib_screen_info {
     cairo_xlib_screen_info_t *next;
     cairo_reference_count_t ref_count;
+    cairo_mutex_t mutex;
 
     cairo_xlib_display_t *display;
     Screen *screen;
diff --git a/src/cairo-xlib-screen.c b/src/cairo-xlib-screen.c
index 8a3da66..fc07fda 100644
--- a/src/cairo-xlib-screen.c
+++ b/src/cairo-xlib-screen.c
@@ -256,12 +256,14 @@ _cairo_xlib_screen_info_close_display (cairo_xlib_screen_info_t *info)
 {
     int i;
 
+    CAIRO_MUTEX_LOCK (info->mutex);
     for (i = 0; i < ARRAY_LENGTH (info->gc); i++) {
 	if (info->gc[i] != NULL) {
 	    XFreeGC (info->display->display, info->gc[i]);
 	    info->gc[i] = NULL;
 	}
     }
+    CAIRO_MUTEX_UNLOCK (info->mutex);
 }
 
 void
@@ -295,6 +297,8 @@ _cairo_xlib_screen_info_destroy (cairo_xlib_screen_info_t *info)
 
     _cairo_array_fini (&info->visuals);
 
+    CAIRO_MUTEX_FINI (info->mutex);
+
     free (info);
 }
 
@@ -335,6 +339,7 @@ _cairo_xlib_screen_info_get (Display *dpy, Screen *screen)
 	info = malloc (sizeof (cairo_xlib_screen_info_t));
 	if (info != NULL) {
 	    CAIRO_REFERENCE_COUNT_INIT (&info->ref_count, 2); /* Add one for display cache */
+	    CAIRO_MUTEX_INIT (info->mutex);
 	    info->display = _cairo_xlib_display_reference (display);
 	    info->screen = screen;
 	    info->has_render = FALSE;
@@ -385,16 +390,18 @@ GC
 _cairo_xlib_screen_get_gc (cairo_xlib_screen_info_t *info, int depth)
 {
     GC gc;
+    cairo_bool_t needs_reset;
 
     depth = depth_to_index (depth);
 
+    CAIRO_MUTEX_LOCK (info->mutex);
     gc = info->gc[depth];
     info->gc[depth] = NULL;
+    needs_reset = info->gc_needs_clip_reset & (1 << depth);
+    CAIRO_MUTEX_UNLOCK (info->mutex);
 
-    if (info->gc_needs_clip_reset & (1 << depth)) {
+    if (needs_reset)
 	XSetClipMask(info->display->display, gc, None);
-	info->gc_needs_clip_reset &= ~(1 << depth);
-    }
 
     return gc;
 }
@@ -403,21 +410,25 @@ cairo_status_t
 _cairo_xlib_screen_put_gc (cairo_xlib_screen_info_t *info, int depth, GC gc, cairo_bool_t reset_clip)
 {
     cairo_status_t status = CAIRO_STATUS_SUCCESS;
+    GC oldgc;
 
     depth = depth_to_index (depth);
 
-    if (info->gc[depth] != NULL) {
-	status = _cairo_xlib_display_queue_work (info->display,
-		                               (cairo_xlib_notify_func) XFreeGC,
-					       info->gc[depth],
-					       NULL);
-    }
-
+    CAIRO_MUTEX_LOCK (info->mutex);
+    oldgc = info->gc[depth];
     info->gc[depth] = gc;
     if (reset_clip)
 	info->gc_needs_clip_reset |= 1 << depth;
     else
 	info->gc_needs_clip_reset &= ~(1 << depth);
+    CAIRO_MUTEX_UNLOCK (info->mutex);
+
+    if (oldgc != NULL) {
+	status = _cairo_xlib_display_queue_work (info->display,
+		                               (cairo_xlib_notify_func) XFreeGC,
+					       oldgc,
+					       NULL);
+    }
 
     return status;
 }
-- 
1.5.5.rc0.24.g86c30

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.cairographics.org/archives/cairo/attachments/20080411/67fc7b34/attachment.pgp 


More information about the cairo mailing list