Hello Matthieu,
On Nov 01 2022, Matthieu Herrb wrote:
> Hi,
>
> here's a libX11 patch that needs some wide testing, especially from
> people who have experienced issues with various applications (fvwm[23]
> from ports, Motif applications with Drag'n'Drop,..) with the upgrade
> to libX11 1.8.1, before I added the --disable-thread-safety-constructor
> option to the build.
>
> This patch allows the callback functions from X*IfEvent() class of
> Xlib functions to re-enter libX11, by making the
> XlockDisplay()/XunlockDisplay() functions void while running the
> callbacks.
>
> I've tested fvwm2 with this patch and it seems to fix it (but since
> the issue was never easy to reproduce I'm not 100% confident).
>
> Please apply and report
For me fvwm2 and fvwm3 crash again.
The bug is the one I explained to you in a private message:
> I'm able to make fvwm2 and fvwm3 to abort, in this case with a clean X
> exit, by including any xapp with the -iconic option in the InitFunction or
> the StartFuncion:
>
> AddToFunc InitFunction I Exec exec xconsole -iconic
>
> Just removing the "-iconic" option it doesn't happen. Besides,
> iconifying to FvwmIconMan doesn't do the trick, it's necessary to NOT
> have the following option enabled (as is the case in latest default
> fvwm2 config):
>
> #Style * !Icon
>
>
> So, unless it happens only to me, as always, you should reproduce it
> just coping the default fvwm2 config to your home directory and adding
> three lines.
>
> $ cp -r /usr/locar/share/fvwm/default-config ~/.fvwm
> $ echo "AddToFunc InitFunction I Exec exec xconsole -iconic" >> ~/.fvwm/config
> $ echo "Style * Icon" >> ~/.fvwm/config
***
>
> cd /usr/xenocara/lib/libX11
> patch -p0 -E < /path/to/this/patch
> doas make -f Makefile.bsd-wrapper obj
> doas make -f Makefile.bsd-wrapper build
>
> Then restart X with the applications that where crashing last august.
>
> Index: Makefile.bsd-wrapper
> ===================================================================
> RCS file: /cvs/OpenBSD/xenocara/lib/libX11/Makefile.bsd-wrapper,v
> retrieving revision 1.29
> diff -u -p -u -r1.29 Makefile.bsd-wrapper
> --- Makefile.bsd-wrapper 3 Sep 2022 06:55:25 -0000 1.29
> +++ Makefile.bsd-wrapper 1 Nov 2022 09:21:34 -0000
> @@ -14,7 +14,6 @@ SHARED_LIBS= X11 18.0 X11_xcb 2.0
>
> CONFIGURE_ARGS= --enable-tcp-transport --enable-unix-transport --enable-ipv6
> \
> --disable-composecache \
> - --disable-thread-safety-constructor \
> --without-xmlto --without-fop --without-xsltproc
>
> .include <bsd.xorg.mk>
> Index: include/X11/Xlibint.h
> ===================================================================
> RCS file: /cvs/OpenBSD/xenocara/lib/libX11/include/X11/Xlibint.h,v
> retrieving revision 1.15
> diff -u -p -u -r1.15 Xlibint.h
> --- include/X11/Xlibint.h 21 Feb 2022 08:01:24 -0000 1.15
> +++ include/X11/Xlibint.h 1 Nov 2022 09:21:34 -0000
> @@ -207,6 +207,7 @@ struct _XDisplay
>
> XIOErrorExitHandler exit_handler;
> void *exit_handler_data;
> + Bool in_ifevent;
> };
>
> #define XAllocIDs(dpy,ids,n) (*(dpy)->idlist_alloc)(dpy,ids,n)
> Index: src/ChkIfEv.c
> ===================================================================
> RCS file: /cvs/OpenBSD/xenocara/lib/libX11/src/ChkIfEv.c,v
> retrieving revision 1.4
> diff -u -p -u -r1.4 ChkIfEv.c
> --- src/ChkIfEv.c 30 May 2011 19:19:38 -0000 1.4
> +++ src/ChkIfEv.c 1 Nov 2022 09:21:35 -0000
> @@ -50,6 +50,7 @@ Bool XCheckIfEvent (
> int n; /* time through count */
>
> LockDisplay(dpy);
> + dpy->in_ifevent = True;
> prev = NULL;
> for (n = 3; --n >= 0;) {
> for (qelt = prev ? prev->next : dpy->head;
> @@ -60,6 +61,7 @@ Bool XCheckIfEvent (
> *event = qelt->event;
> _XDeq(dpy, prev, qelt);
> _XStoreEventCookie(dpy, event);
> + dpy->in_ifevent = False;
> UnlockDisplay(dpy);
> return True;
> }
> @@ -78,6 +80,7 @@ Bool XCheckIfEvent (
> /* another thread has snatched this event */
> prev = NULL;
> }
> + dpy->in_ifevent = False;
> UnlockDisplay(dpy);
> return False;
> }
> Index: src/IfEvent.c
> ===================================================================
> RCS file: /cvs/OpenBSD/xenocara/lib/libX11/src/IfEvent.c,v
> retrieving revision 1.4
> diff -u -p -u -r1.4 IfEvent.c
> --- src/IfEvent.c 30 May 2011 19:19:38 -0000 1.4
> +++ src/IfEvent.c 1 Nov 2022 09:21:35 -0000
> @@ -49,6 +49,7 @@ XIfEvent (
> unsigned long qe_serial = 0;
>
> LockDisplay(dpy);
> + dpy->in_ifevent = True;
> prev = NULL;
> while (1) {
> for (qelt = prev ? prev->next : dpy->head;
> @@ -59,6 +60,7 @@ XIfEvent (
> *event = qelt->event;
> _XDeq(dpy, prev, qelt);
> _XStoreEventCookie(dpy, event);
> + dpy->in_ifevent = False;
> UnlockDisplay(dpy);
> return 0;
> }
> Index: src/OpenDis.c
> ===================================================================
> RCS file: /cvs/OpenBSD/xenocara/lib/libX11/src/OpenDis.c,v
> retrieving revision 1.12
> diff -u -p -u -r1.12 OpenDis.c
> --- src/OpenDis.c 28 Nov 2020 14:39:48 -0000 1.12
> +++ src/OpenDis.c 1 Nov 2022 09:21:35 -0000
> @@ -189,6 +189,7 @@ XOpenDisplay (
> dpy->xcmisc_opcode = 0;
> dpy->xkb_info = NULL;
> dpy->exit_handler_data = NULL;
> + dpy->in_ifevent = False;
>
> /*
> * Setup other information in this display structure.
> Index: src/PeekIfEv.c
> ===================================================================
> RCS file: /cvs/OpenBSD/xenocara/lib/libX11/src/PeekIfEv.c,v
> retrieving revision 1.3
> diff -u -p -u -r1.3 PeekIfEv.c
> --- src/PeekIfEv.c 30 May 2011 19:19:38 -0000 1.3
> +++ src/PeekIfEv.c 1 Nov 2022 09:21:35 -0000
> @@ -50,6 +50,7 @@ XPeekIfEvent (
> unsigned long qe_serial = 0;
>
> LockDisplay(dpy);
> + dpy->in_ifevent = True;
> prev = NULL;
> while (1) {
> for (qelt = prev ? prev->next : dpy->head;
> @@ -63,6 +64,7 @@ XPeekIfEvent (
> _XStoreEventCookie(dpy, ©);
> *event = copy;
> }
> + dpy->in_ifevent = False;
> UnlockDisplay(dpy);
> return 0;
> }
> Index: src/locking.c
> ===================================================================
> RCS file: /cvs/OpenBSD/xenocara/lib/libX11/src/locking.c,v
> retrieving revision 1.8
> diff -u -p -u -r1.8 locking.c
> --- src/locking.c 28 Nov 2020 14:39:48 -0000 1.8
> +++ src/locking.c 1 Nov 2022 09:21:35 -0000
> @@ -455,6 +455,32 @@ static void _XDisplayLockWait(
> static void _XLockDisplay(
> Display *dpy
> XTHREADS_FILE_LINE_ARGS
> + );
> +
> +static void _XIfEventLockDisplay(
> + Display *dpy
> + XTHREADS_FILE_LINE_ARGS
> + )
> +{
> + /* assert(dpy->in_ifevent); */
> +}
> +
> +static void _XIfEventUnlockDisplay(
> + Display *dpy
> + XTHREADS_FILE_LINE_ARGS
> + )
> +{
> + if (dpy->in_ifevent)
> + return;
> +
> + dpy->lock_fns->lock_display = _XLockDisplay;
> + dpy->lock_fns->unlock_display = _XUnlockDisplay;
> + UnlockDisplay(dpy);
> +}
> +
> +static void _XLockDisplay(
> + Display *dpy
> + XTHREADS_FILE_LINE_ARGS
> )
> {
> #ifdef XTHREADS
> @@ -478,6 +504,10 @@ static void _XLockDisplay(
> #endif
> _XIDHandler(dpy);
> _XSeqSyncFunction(dpy);
> + if (dpy->in_ifevent) {
> + dpy->lock_fns->lock_display = _XIfEventLockDisplay;
> + dpy->lock_fns->unlock_display = _XIfEventUnlockDisplay;
> + }
> }
>
> /*
>
> --
> Matthieu Herrb
>
>
--
Walter
