Package: rxvt Version: 1:2.7.10-7 Severity: normal Tags: patch Dear Maintainer,
Here are two patches, see the git commit message for more details. rxvt is incorrectly using sizeof to calculate the format for XChangeProperty, getting 64 on a 64 bit system, when 8, 16, 32 are the only options. This results in rxvt terminating when it receives a request for a TIMESTAMP on the primary selection. If completely obscured and something requests a draw, rxvt correctly detects that there's no point in drawing, but needs to clear want_refresh to avoid sleeping 5ms and trying again. Both changes are effectively one line code changes. -- System Information: Debian Release: 9.5 APT prefers stable APT policy: (500, 'stable') Architecture: amd64 (x86_64) Foreign Architectures: i386 Kernel: Linux 4.18.0+ (SMP w/4 CPU cores) Locale: LANG=C, LC_CTYPE=en_US.ISO-8859-15 (charmap=ISO-8859-15), LANGUAGE=C (charmap=ISO-8859-15) Shell: /bin/sh linked to /bin/dash Init: systemd (via /run/systemd/system) Versions of packages rxvt depends on: ii libc6 2.24-11+deb9u3 ii libx11-6 2:1.6.4-3 ii libxpm4 1:3.5.12-1 rxvt recommends no packages. rxvt suggests no packages. -- no debconf information
>From f03a0f43a92bb06acefe29a62b50001218f4c45a Mon Sep 17 00:00:00 2001 From: David Fries <da...@fries.net> Date: Sat, 29 Sep 2018 22:26:43 -0500 Subject: [PATCH 1/3] fix XA_TIMESTAMP XChangeProperty format value If rxvt owns the primary selection buffer and another program queries the TIMESTAMP as the target to XConvertSelection, rxvt will exit with the following X error (after disabling rxvt's error handler to let the built in handler decode it). X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 18 (X_ChangeProperty) Value in failed request: 0x40 Serial number of failed request: 232 Current serial number in output stream: 233 The failed value is 0x40 which is 64, which is (8 * sizeof(Time)) on a 64 bit system, but the man page to XChangeProperty property states, "Possible values are 8, 16, and 32." This is an integer, set the value to 32. source code to a simple program to request the TIMESTAMP from the primary selection owner can be found here, https://gitlab.com/dfries64/xconvertselection_timestamp --- src/screen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/screen.c b/src/screen.c index 5f3ed75..dd5def7 100644 --- a/src/screen.c +++ b/src/screen.c @@ -3647,7 +3647,7 @@ rxvt_selection_send(rxvt_t *r, const XSelectionRequestEvent *rq) /* TODO: Handle MULTIPLE */ } else if (rq->target == r->h->xa[XA_TIMESTAMP] && r->selection.text) { XChangeProperty(r->Xdisplay, rq->requestor, rq->property, XA_INTEGER, - (8 * sizeof(Time)), PropModeReplace, + 32, PropModeReplace, (unsigned char *)&r->h->selection_time, 1); ev.property = rq->property; } else if (rq->target == XA_STRING -- 2.11.0 >From 1f5325b8c546a859d4f00a02292656187e19f8e4 Mon Sep 17 00:00:00 2001 From: David Fries <da...@fries.net> Date: Thu, 27 Sep 2018 22:47:51 -0500 Subject: [PATCH 2/3] rxvt eats cpu when needing to draw while obscured If the window is completely obscured and needs to draw then rxvt_scr_refresh will return immediately instead of drawing, (no need to draw what can't be seen), but if want_refresh is true, it will do a 5ms select timeout and try again forever. The solution is to clear want_refresh before returning from rxvt_scr_refresh. To reproduce run `strace rxvt`, then `top` in that rxvt window (or anything with a delay that will trigger a draw, `sleep 5` works), completely cover the rxvt window with another and wait for the timeout. strace will show a steady stream of select (which times out), and recvmsg. --- src/screen.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/screen.c b/src/screen.c index dd5def7..d637930 100644 --- a/src/screen.c +++ b/src/screen.c @@ -2046,8 +2046,15 @@ rxvt_scr_refresh(rxvt_t *r, unsigned char refresh_type) int (*draw_string_func) () = draw_string; int (*draw_image_string_func) () = draw_image_string; - if (refresh_type == NO_REFRESH || !r->TermWin.mapped) + if (refresh_type == NO_REFRESH || !r->TermWin.mapped) { + /* If the window is completely obscured (NO_REFRESH) we really don't + * want to refresh, that is why we are returning instead, but first we + * must say mark the flag don't refresh. A similar argument works for + * !mapped. + */ + h->want_refresh = 0; return; + } /* * A: set up vars -- 2.11.0 >From f12ebf710079400f44e9b16fc0bf7c2a9635fd1c Mon Sep 17 00:00:00 2001 From: David Fries <da...@fries.net> Date: Wed, 26 Sep 2018 21:55:47 -0500 Subject: [PATCH 3/3] change long with hang, hang, X error termination, wasting cpu --- debian/changelog | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/debian/changelog b/debian/changelog index 35341f9..27333ac 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,18 @@ +rxvt (1:2.7.10-7nohang) unstable; urgency=medium + + * Fixed rxvt hang on exit, seteuid() gets a mutex, is interrupted with + SIGCHLD signal, calls the same exit path and seteuid() blocks waiting for + mutex + * Fixed rxvt hang on exit, malloc gets a mutex, is interrupted with + SIGCHLD signal, rxvt_clean_exit calls XDestroyIC(), and free() blocks + waiting for mutex + * fix termination due to X error, a XA_TIMESTAMP query called XChangeProperty + with 64 instead of 32 + * fix looping with a 5ms timeout when needing to draw while completely + obscured + + -- David Fries <da...@fries.net> Wed, 26 Sep 2018 21:55:35 -0500 + rxvt (1:2.7.10-7) unstable; urgency=medium * Fixed rxvt-ml cjk builds to use updated configure params. -- 2.11.0
