Fix multithreaded crash with tiled regions.

Michael has found a crash when scrolling quickly through pages
with gsview. 2 Threads are redrawing at the same time from a
display list. The problem comes when both threads happen to be
trying to draw the same tile from the cache at the same time.

The current code alters the ->{x,y} values of the pixmap from
the cache as it tiles. If 2 threads are using the same tile
at the same time, this causes a race condition which can upset
the clipping calculations and we can access out of range.

The solution is to make a new 'wrapper' fz_pixmap around the
same data, and to alter the x/y values there instead.

We therefore introduce a (hopefully generally useful) function
fz_new_pixmap_from_pixmap, and use that.

2 files changed, 79 insertions, 36 deletions

diff --git a/source/fitz/draw-device.c b/source/fitz/draw-device.c
index 03351940..8686c7dd 100644
--- a/source/fitz/draw-device.c
+++ b/source/fitz/draw-device.c
@@ -2692,6 +2692,9 @@ fz_draw_end_tile(fz_context *ctx, fz_device *devp)
 	fz_rect scissor_tmp, tile_tmp;
 	int x0, y0, x1, y1, x, y, extra_x, extra_y;
 	fz_draw_state *state;
+	fz_pixmap *dest = NULL;
+	fz_pixmap *shape = NULL;
+	fz_pixmap *group_alpha = NULL;
 
 	if (dev->top == 0)
 	{
@@ -2764,52 +2767,57 @@ fz_draw_end_tile(fz_context *ctx, fz_device *devp)
 	if (state[0].group_alpha)
 		fz_dump_blend(ctx, "/GA=", state[0].group_alpha);
 #endif
+	dest = fz_new_pixmap_from_pixmap(ctx, state[1].dest, NULL);
+
+	fz_var(shape);
+	fz_var(group_alpha);
 
-	for (y = y0; y < y1; y++)
+	fz_try(ctx)
 	{
-		for (x = x0; x < x1; x++)
+		shape = fz_new_pixmap_from_pixmap(ctx, state[1].shape, NULL);
+		group_alpha = fz_new_pixmap_from_pixmap(ctx, state[1].group_alpha, NULL);
+
+		for (y = y0; y < y1; y++)
 		{
-			ttm = ctm;
-			fz_pre_translate(&ttm, x * xstep, y * ystep);
-			state[1].dest->x = ttm.e;
-			state[1].dest->y = ttm.f;
-			/* Check for overflow due to float -> int conversions */
-			if (state[1].dest->x > 0 && state[1].dest->x + state[1].dest->w < 0)
-				continue;
-			if (state[1].dest->y > 0 && state[1].dest->y + state[1].dest->h < 0)
-				continue;
-			fz_paint_pixmap_with_bbox(state[0].dest, state[1].dest, 255, state[0].scissor);
-			if (state[1].shape)
+			for (x = x0; x < x1; x++)
 			{
-				ttm = shapectm;
+				ttm = ctm;
 				fz_pre_translate(&ttm, x * xstep, y * ystep);
-				state[1].shape->x = ttm.e;
-				state[1].shape->y = ttm.f;
-				fz_paint_pixmap_with_bbox(state[0].shape, state[1].shape, 255, state[0].scissor);
-			}
-			if (state[1].group_alpha)
-			{
-				ttm = gactm;
-				fz_pre_translate(&ttm, x * xstep, y * ystep);
-				state[1].group_alpha->x = ttm.e;
-				state[1].group_alpha->y = ttm.f;
-				fz_paint_pixmap_with_bbox(state[0].group_alpha, state[1].group_alpha, 255, state[0].scissor);
+				dest->x = ttm.e;
+				dest->y = ttm.f;
+				/* Check for overflow due to float -> int conversions */
+				if (dest->x > 0 && dest->x + dest->w < 0)
+					continue;
+				if (dest->y > 0 && dest->y + dest->h < 0)
+					continue;
+				fz_paint_pixmap_with_bbox(state[0].dest, dest, 255, state[0].scissor);
+				if (shape)
+				{
+					ttm = shapectm;
+					fz_pre_translate(&ttm, x * xstep, y * ystep);
+					shape->x = ttm.e;
+					shape->y = ttm.f;
+					fz_paint_pixmap_with_bbox(state[0].shape, shape, 255, state[0].scissor);
+				}
+				if (group_alpha)
+				{
+					ttm = gactm;
+					fz_pre_translate(&ttm, x * xstep, y * ystep);
+					group_alpha->x = ttm.e;
+					group_alpha->y = ttm.f;
+					fz_paint_pixmap_with_bbox(state[0].group_alpha, group_alpha, 255, state[0].scissor);
+				}
 			}
 		}
 	}
-
-	state[1].dest->x = ctm.e;
-	state[1].dest->y = ctm.f;
-	if (state[1].shape)
-	{
-		state[1].shape->x = shapectm.e;
-		state[1].shape->y = shapectm.f;
-	}
-	if (state[1].group_alpha)
+	fz_always(ctx)
 	{
-		state[1].group_alpha->x = gactm.e;
-		state[1].group_alpha->y = gactm.f;
+		fz_drop_pixmap(ctx, dest);
+		fz_drop_pixmap(ctx, shape);
+		fz_drop_pixmap(ctx, group_alpha);
 	}
+	fz_catch(ctx)
+		fz_rethrow(ctx);
 
 	/* Now we try to cache the tiles. Any failure here will just result in us not caching. */
 	if (state[1].encache && state[1].id != 0)
diff --git a/source/fitz/pixmap.c b/source/fitz/pixmap.c
index 4e3f62cb..93f8fed6 100644
--- a/source/fitz/pixmap.c
+++ b/source/fitz/pixmap.c
@@ -26,6 +26,7 @@ fz_drop_pixmap_imp(fz_context *ctx, fz_storable *pix_)
 	fz_drop_separations(ctx, pix->seps);
 	if (pix->flags & FZ_PIXMAP_FLAG_FREE_SAMPLES)
 		fz_free(ctx, pix->samples);
+	fz_drop_pixmap(ctx, pix->underlying);
 	fz_free(ctx, pix);
 }
 
@@ -126,6 +127,40 @@ fz_new_pixmap_with_bbox_and_data(fz_context *ctx, fz_colorspace *colorspace, con
 	return pixmap;
 }
 
+fz_pixmap *fz_new_pixmap_from_pixmap(fz_context *ctx, fz_pixmap *pixmap, const fz_irect *rect)
+{
+	fz_irect local_rect;
+	fz_pixmap *subpix;
+
+	if (!pixmap)
+		return NULL;
+
+	if (rect == NULL)
+	{
+		rect = &local_rect;
+		local_rect.x0 = pixmap->x;
+		local_rect.y0 = pixmap->y;
+		local_rect.x1 = pixmap->x + pixmap->w;
+		local_rect.y1 = pixmap->y + pixmap->h;
+	}
+	else if (rect->x0 < pixmap->x || rect->y0 < pixmap->y || rect->x1 > pixmap->x + pixmap->w || rect->y1 > pixmap->y + pixmap->h)
+		fz_throw(ctx, FZ_ERROR_GENERIC, "Pixmap region is not a subarea");
+
+	subpix = fz_malloc_struct(ctx, fz_pixmap);
+	*subpix = *pixmap;
+	subpix->x = rect->x0;
+	subpix->y = rect->y0;
+	subpix->w = rect->x1 - rect->x0;
+	subpix->h = rect->y1 - rect->y0;
+	subpix->samples += (rect->x0 - pixmap->x) + (rect->y0 - pixmap->y) * pixmap->stride;
+	subpix->underlying = fz_keep_pixmap(ctx, pixmap);
+	subpix->colorspace = fz_keep_colorspace(ctx, pixmap->colorspace);
+	subpix->seps = fz_keep_separations(ctx, pixmap->seps);
+	subpix->flags &= ~FZ_PIXMAP_FLAG_FREE_SAMPLES;
+
+	return subpix;
+}
+
 fz_irect *
 fz_pixmap_bbox(fz_context *ctx, const fz_pixmap *pix, fz_irect *bbox)
 {