Hi Matthiew,

On Nov 11 2022, Walter Alejandro Iglesias wrote:
> On Nov 11 2022, Matthieu Herrb wrote:
> > 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...
> 
> This patch solved the firefox cwm issue.  Firefox shows this when
> it starts: 
> 
>   ~$ firefox
>   XXX _XLockDisplay 1
>   XXX _XIfEventInternalLockDisplay 1
>   XXX _XIfEventUnlockDisplay 0
> 
> fvwm2/3 still crashing.

Today, after upgrading to the latest snapshot I can not reproduce this
fvwm bug anymore.  I ignore why.

> 
> 
> 
> > 
> > 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
> 
> -- 
> Walter

-- 
Walter

Reply via email to