Bug 704709 - Don't free the background pixmap created by Ghostview
Summary: Don't free the background pixmap created by Ghostview
Status: RESOLVED FIXED
Alias: None
Product: Ghostscript
Classification: Unclassified
Component: X Display Driver (show other bugs)
Version: 9.54.0
Hardware: PC Linux
: P4 normal
Assignee: Chris Liddell (chrisl)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-11-10 08:32 UTC by Florian Lindemann
Modified: 2021-11-29 12:02 UTC (History)
1 user (show)

See Also:
Customer:
Word Size: ---


Attachments
Patch: Don't free the background pixmap created by Ghostview (738 bytes, patch)
2021-11-10 08:32 UTC, Florian Lindemann
Details | Diff
Only free pixmap if xdev->ghostview is NOT set (525 bytes, patch)
2021-11-26 07:53 UTC, Florian Lindemann
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Lindemann 2021-11-10 08:32:54 UTC
Created attachment 21872 [details]
Patch: Don't free the background pixmap created by Ghostview

Commit 033ed8bf196b1cddd681a9b32d9398bf6bc02d24 introduced a call to XFreePixmap in gdev_x_close.
This might free a pixmap that was created by ghostview as a backing
pixmap and might still be used by ghostview for further drawing.

I'm not entirely sure which files causes the gs process to close the device
early (i.e. before ghostview quits), but with some files it does and gv actions like reloading/zooming attempt to draw on the already freed pixmap and cause gv to crash with

X Error of failed request:  BadPixmap (invalid Pixmap parameter).

Especially the -watch option of gv is broken if it is used to watch a
postscript file that's in development. Any postscript error will cause the
device to free the backing pixmap and ghostview will crash even if the
error in the file gets fixed. It was possible to do that in the past (the
backing pixmap was reused with a new gs process).

It's possible to reproduce the issue with a small example file like this:

newpath
100 200 moveto
200 250 lineto
100 300 lineto
2 setlinewidth
stroke

Please note that there is no "showpage", which seems to cause the invocation
of gdev_x_close (?) (with showpage, the gs process keeps running). With such
a file gv will crash when trying to zoom / reload etc. (ghostview needs to run
with backing pixmap enabled (-pixmap), the -nopixmap option will of course
work around the bug).

I attached a simple patch that checks if the pixmap is created by ghostview
(xdev->ghostview is set) and only frees it if that's not the case.

Cheers,
Florian
Comment 1 Chris Liddell (chrisl) 2021-11-10 14:23:03 UTC
Slightly tweaked patch applied:

https://git.ghostscript.com/?p=ghostpdl.git;a=commitdiff;h=d468a903


Thanks for the contribution!
Comment 2 Florian Lindemann 2021-11-26 07:53:30 UTC
Created attachment 21899 [details]
Only free pixmap if xdev->ghostview is NOT set
Comment 3 Florian Lindemann 2021-11-26 07:57:56 UTC
Dear Chris,

i only just noticed that your changes unfortunately inverted the logic of my original patch. The issue remains unresolved.

Regards,
Florian

(In reply to Florian Lindemann from comment #2)
> Created attachment 21899 [details]
> Only free pixmap if xdev->ghostview is NOT set
Comment 4 Chris Liddell (chrisl) 2021-11-26 11:33:25 UTC
(In reply to Florian Lindemann from comment #3)
> Dear Chris,
> 
> i only just noticed that your changes unfortunately inverted the logic of my
> original patch. The issue remains unresolved.
> 
> Regards,
> Florian


Oops! Sorry about that - I'll sort it out when I'm back at my desk.
Comment 5 Chris Liddell (chrisl) 2021-11-29 12:02:32 UTC
Hopefully correct now:

https://git.ghostscript.com/?p=ghostpdl.git;a=commitdiff;h=30c7908165b0