On Fri, Nov 11, 2022 at 03:17:21PM +0100, Walter Alejandro Iglesias wrote:
> On Nov 11 2022, Matthieu Herrb wrote:
> > Hi,
> > 
> > So the patch provided by Adam Jackson upstreams is completely buggy.
> > 
> > - the logic to setup the new locking function was busted
> > - I've observed  cases where even the XGetIfEvent() function gets
> >   re-rentered. So the flags does in fact need to be a counter.
> > 
> > New patch which works for me with fvwm2...
> > 
> > Testing is welcome...
> 
> Unfortunately I still can reproduce the bug.
> 
> This patch also makes firefox crash, (I'm not sure if it's because of
> firefox or cwm).  After reinstalling xenocara from the snapshots
> packages I could run firefox again.  
> 
> fvwm is giving a lot of problems lately, there's also this drm bug I
> reported: 
> 
>   https://marc.info/?l=openbsd-bugs&m=166332904632232&w=2
> 
> Now FvwmIcons is crashing on fvwm3...  I'm giving up with fvwm, I'm more
> comfortable with cwm.

Ok, I've figured out the cwm + firefox issue. There's also a
XInternalLock that needs to be neutered in the X*IfEvent() callbacks.

New diff, on which I've left my debug printfs. If you get other
crashes it may be interesting to look in .xsession-errors...

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        11 Nov 2022 15:13:59 -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       11 Nov 2022 15:14:00 -0000
@@ -207,6 +207,7 @@ struct _XDisplay
 
        XIOErrorExitHandler exit_handler;
        void *exit_handler_data;
+        CARD32 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       11 Nov 2022 15:14:00 -0000
@@ -49,6 +49,7 @@ Bool XCheckIfEvent (
        unsigned long qe_serial = 0;
        int n;                  /* time through count */
 
+        dpy->in_ifevent++;
         LockDisplay(dpy);
        prev = NULL;
        for (n = 3; --n >= 0;) {
@@ -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--;
        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       11 Nov 2022 15:14:00 -0000
@@ -48,6 +48,7 @@ XIfEvent (
        register _XQEvent *qelt, *prev;
        unsigned long qe_serial = 0;
 
+        dpy->in_ifevent++;
         LockDisplay(dpy);
        prev = NULL;
        while (1) {
@@ -59,6 +60,7 @@ XIfEvent (
                    *event = qelt->event;
                    _XDeq(dpy, prev, qelt);
                    _XStoreEventCookie(dpy, event);
+                    dpy->in_ifevent--;
                    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       11 Nov 2022 15:14:00 -0000
@@ -189,6 +189,7 @@ XOpenDisplay (
        dpy->xcmisc_opcode      = 0;
        dpy->xkb_info           = NULL;
        dpy->exit_handler_data  = NULL;
+        dpy->in_ifevent         = 0;
 
 /*
  * 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      11 Nov 2022 15:14:00 -0000
@@ -49,6 +49,7 @@ XPeekIfEvent (
        register _XQEvent *prev, *qelt;
        unsigned long qe_serial = 0;
 
+        dpy->in_ifevent++;
        LockDisplay(dpy);
        prev = NULL;
        while (1) {
@@ -63,6 +64,7 @@ XPeekIfEvent (
                        _XStoreEventCookie(dpy, &copy);
                        *event = copy;
                    }
+                    dpy->in_ifevent--;
                    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       11 Nov 2022 15:14:00 -0000
@@ -40,6 +40,7 @@ in this Software without prior written a
 #undef _XFreeMutex
 
 #ifdef XTHREADS
+#include <stdio.h>
 
 #ifdef __UNIXWARE__
 #include <dlfcn.h>
@@ -455,6 +456,51 @@ static void _XDisplayLockWait(
 static void _XLockDisplay(
     Display *dpy
     XTHREADS_FILE_LINE_ARGS
+    );
+
+static void _XIfEventLockDisplay(
+    Display *dpy
+    XTHREADS_FILE_LINE_ARGS
+    )
+{
+    printf("XXX %s %d\n", __func__, dpy->in_ifevent);
+    /* assert(dpy->in_ifevent); */
+}
+
+static void _XInternalLockDisplay(
+    Display *dpy,
+    Bool wskip
+    XTHREADS_FILE_LINE_ARGS
+    );
+
+static void _XIfEventInternalLockDisplay(
+    Display *dpy,
+    Bool wskip
+    XTHREADS_FILE_LINE_ARGS
+    )
+{
+    printf("XXX %s %d\n", __func__, dpy->in_ifevent);
+}
+
+static void _XIfEventUnlockDisplay(
+    Display *dpy
+    XTHREADS_FILE_LINE_ARGS
+    )
+{
+    setbuf(stdout, NULL);
+    if (dpy->in_ifevent == 0) {
+        printf("XXX %s %d\n", __func__, dpy->in_ifevent);
+        dpy->lock_fns->lock_display = _XLockDisplay;
+        dpy->lock_fns->unlock_display = _XUnlockDisplay;
+        dpy->lock->internal_lock_display = _XInternalLockDisplay;
+        UnlockDisplay(dpy);
+    } else
+        return;
+}
+
+static void _XLockDisplay(
+    Display *dpy
+    XTHREADS_FILE_LINE_ARGS
     )
 {
 #ifdef XTHREADS
@@ -478,6 +524,12 @@ static void _XLockDisplay(
 #endif
     _XIDHandler(dpy);
     _XSeqSyncFunction(dpy);
+    if (dpy->in_ifevent) {
+        printf("XXX %s %d\n", __func__, dpy->in_ifevent);
+        dpy->lock_fns->lock_display = _XIfEventLockDisplay;
+        dpy->lock_fns->unlock_display = _XIfEventUnlockDisplay;
+        dpy->lock->internal_lock_display = _XIfEventInternalLockDisplay;
+    }
 }
 
 /*

-- 
Matthieu Herrb

Reply via email to