Re: Fwd: XInitThreads in library constructor breaks Motif!

2022-11-01 Thread Po Lu



On November 2, 2022 1:25:06 AM GMT+08:00, Adam Jackson  wrote:
>On Mon, Oct 31, 2022 at 8:46 PM Po Lu  wrote:

>Again, I proposed a workaround that preserves the thread-safety guarantee I
>need, and wrote the patch implementing it. Sure would be nice if someone
>tested it.

I did say it works for the programs that broke for me.


Re: Fwd: XInitThreads in library constructor breaks Motif!

2022-11-01 Thread Matthieu Herrb
On Tue, Nov 01, 2022 at 01:25:06PM -0400, Adam Jackson wrote:
> On Mon, Oct 31, 2022 at 8:46 PM Po Lu  wrote:
> 
> > Adam Jackson  writes:
> >
> > > Because XInitThreads only works if it is called before XOpenDisplay.
> >
> > So why not fix that?
> >
> > I think the simple fix would be to call _XInitDisplayLock in
> > XGetXCBConnection.
> >
> 
> That wouldn't be sufficient, off the top of my head there's library-global
> state that gets set up in XInitThreads too and none of the display-level
> locking will work without it.
> 
> Again, I proposed a workaround that preserves the thread-safety guarantee I
> need, and wrote the patch implementing it. Sure would be nice if someone
> tested it.
> 

Hi,

I'm trying to get it tested on OpenBSD where some people complained
after the import of libX11 1.8.1:

https://marc.infœ?l=openbsd-tech=166729483920618=2

On my small test case it seems to fix the issue.
-- 
Matthieu Herrb


Re: XInitThreads in library constructor breaks Motif!

2022-11-01 Thread Carsten Haitzler
On Tue, 01 Nov 2022 18:32:38 +0800 Po Lu  said:

> Carsten Haitzler  writes:
> 
> > Sorry - but no. Motif being non-threadsafe and Xlib being threadsafe or not
> > are different matters. A higher level lib that is not threadsafe makes use
> > of a lower level one (xlib) and the latter can be threadsafe if desired.
> 
> Being thread-safe is one thing.  Deadlocking clients is another.  If
> Xlib calls XInitThreads in a library constructor without consequences to
> programs 

i think we have covered that pointing to the mr/pr.

> > They man xinitthreads() then use motif from a single thread. Xlib can
> > be used then from multiple threads (e.g. one motif handled window and
> > another xlib handled window which is drawn to in another thread but
> > the same xlib display and connect below it all - as long as you are
> > not consuming events from both threads).
> 
> Then any program doing that cannot use Motif.  How simple is that?
> 
> > ??? Did you not read what I wrote? You are saying I said the opposite of
> > what I clearly wrote - xcb has nothing to do with discussing xinitthreads
> > being validly enabled or not by default. this really has nothing to do with
> > xcb. xlib being built on xcb is just a happenstance of how xlib is built
> > today and modern code can actually mix and match xcb and xlib, but it's not
> > part of the core issue.
> 
> Evidently, the author of the change in question disagrees.  Motif is not
> safe to use after calling XInitThreads, and the Motif applications on my
> desktop don't do that.  Yet, without doing so, they were frozen, as I
> found out trying to make a calendar appointment.

they are making that argument. i am not. xinitthreads being automatically
called/enabled and xcb are separate things. we can have this discussion
entirely devoid of xcb existing. should threadsafe xlib be a default anyway?
why not? the mr in this thread addresses the deadlock but x11r6 breaks things
anyway so there is certainly room for x11r6 breaking vs x11r5. it certainly
broke parts of icccm.

> And if you think no application today can avoid pulling in EGL or GLX,
> you're just wrong.

I don't think that and never said or implied it in any way.

> > And X11R6 added it.
> 
> Behind XInitThreads.  The old behavior of the library was intentionally
> kept for clients that did not use threads.
> 
>   (anecdotally, Emacs also did not require changes to work on R6.
>Despite being a big and complex program that heavily mixed Xt and
>Xlib code)
> 
> > various things broke/changed between x11r5 and x11r6.  Certainly parts
> > of ICCCM broke that required minor code changes in WMs and some
> > clients. the ICCCM has a nice list of them if you want to look it
> > up.
> 
> And which of those changes caused clients to freeze?

they would have caused other misbehavior or breakages like focus not
necessarily working right for example.

> In fact, which of those changes affected Xlib more than "new fields
> added to XSizeHints, which was documented as subject to extension"?
> 
> > They are not "new features to make use of voluntarily". They are
> > hard "must change your code" changes.
> 
> No, they were "new fields added to structs" changes.  Changes to the
> ICCCM naturally affected clients and window managers, but only if you
> used clients written for the new version of the ICCCM with those written
> for the old one.

one such example was using currenttime as opposed to the event timestamp for
XSetInputFocus (i ran into this myself at some point - reading old code
that used currenttime and then found r6 on needed to not do that). this would
require actual code changes to fix. the point remains the same. x11r6 broke
things that require either compiletime or runtime if/#if's.

> Motif programs, OTOH, were simply broken by Xlib 1.8.0, regardless of
> what other libraries they used.
> 
> > I remember dealing with this back then as there were still some x11r5
> > systems floating about that had some subtle differences. You're now
> > hitting another subtle change only really found now decades
> > later. There was obvious appetite for making small code breakages back
> > then and this here falls into that category.
> 
> Deadlocking Motif programs is not a "small change", and you cannot
> seriously compare it to the ICCCM 2.0.

it's pointing out that x11r6 broke things and code needed to be updated - not
just voluntarily opt into a new feature. that break already occurred. the x11r6
contract that says "don't call xlib calls inside this callback" has been around
for decades. this now actually produces a real side effect as opposed to no
visible side effect. the change in contract changed long ago.

-- 
- Codito, ergo sum - "I code, therefore I am" --
Carsten Haitzler - ras...@rasterman.com



Fwd: XInitThreads in library constructor breaks Motif!

2022-11-01 Thread Adam Jackson
On Mon, Oct 31, 2022 at 8:46 PM Po Lu  wrote:

> Adam Jackson  writes:
>
> > Because XInitThreads only works if it is called before XOpenDisplay.
>
> So why not fix that?
>
> I think the simple fix would be to call _XInitDisplayLock in
> XGetXCBConnection.
>

That wouldn't be sufficient, off the top of my head there's library-global
state that gets set up in XInitThreads too and none of the display-level
locking will work without it.

Again, I proposed a workaround that preserves the thread-safety guarantee I
need, and wrote the patch implementing it. Sure would be nice if someone
tested it.

- ajax


Re: XInitThreads in library constructor breaks Motif!

2022-11-01 Thread Po Lu
Carsten Haitzler  writes:

> Sorry - but no. Motif being non-threadsafe and Xlib being threadsafe or not 
> are
> different matters. A higher level lib that is not threadsafe makes use of a
> lower level one (xlib) and the latter can be threadsafe if desired.

Being thread-safe is one thing.  Deadlocking clients is another.  If
Xlib calls XInitThreads in a library constructor without consequences to
programs 

> They man xinitthreads() then use motif from a single thread. Xlib can
> be used then from multiple threads (e.g. one motif handled window and
> another xlib handled window which is drawn to in another thread but
> the same xlib display and connect below it all - as long as you are
> not consuming events from both threads).

Then any program doing that cannot use Motif.  How simple is that?

> ??? Did you not read what I wrote? You are saying I said the opposite of what 
> I
> clearly wrote - xcb has nothing to do with discussing xinitthreads being 
> validly
> enabled or not by default. this really has nothing to do with xcb. xlib being
> built on xcb is just a happenstance of how xlib is built today and modern code
> can actually mix and match xcb and xlib, but it's not part of the core issue.

Evidently, the author of the change in question disagrees.  Motif is not
safe to use after calling XInitThreads, and the Motif applications on my
desktop don't do that.  Yet, without doing so, they were frozen, as I
found out trying to make a calendar appointment.

And if you think no application today can avoid pulling in EGL or GLX,
you're just wrong.

> And X11R6 added it.

Behind XInitThreads.  The old behavior of the library was intentionally
kept for clients that did not use threads.

  (anecdotally, Emacs also did not require changes to work on R6.
   Despite being a big and complex program that heavily mixed Xt and
   Xlib code)

> various things broke/changed between x11r5 and x11r6.  Certainly parts
> of ICCCM broke that required minor code changes in WMs and some
> clients. the ICCCM has a nice list of them if you want to look it
> up.

And which of those changes caused clients to freeze?

In fact, which of those changes affected Xlib more than "new fields
added to XSizeHints, which was documented as subject to extension"?

> They are not "new features to make use of voluntarily". They are
> hard "must change your code" changes.

No, they were "new fields added to structs" changes.  Changes to the
ICCCM naturally affected clients and window managers, but only if you
used clients written for the new version of the ICCCM with those written
for the old one.

Motif programs, OTOH, were simply broken by Xlib 1.8.0, regardless of
what other libraries they used.

> I remember dealing with this back then as there were still some x11r5
> systems floating about that had some subtle differences. You're now
> hitting another subtle change only really found now decades
> later. There was obvious appetite for making small code breakages back
> then and this here falls into that category.

Deadlocking Motif programs is not a "small change", and you cannot
seriously compare it to the ICCCM 2.0.


Re: XInitThreads in library constructor breaks Motif!

2022-11-01 Thread Carsten Haitzler
On Tue, 01 Nov 2022 08:55:50 +0800 Po Lu  said:

> Carsten Haitzler  writes:
> 
> > no it doesn't. the contract is simply to not use motif from multiple
> > threads. it has nothing to do with xlib itself becoming threadsafe. i think
> > the PR/MR for xlib has all the details you want listed in it.
> 
> No, it doesn't, see below:

Sorry - but no. Motif being non-threadsafe and Xlib being threadsafe or not are
different matters. A higher level lib that is not threadsafe makes use of a
lower level one (xlib) and the latter can be threadsafe if desired. They man
xinitthreads() then use motif from a single thread. Xlib can be used then from
multiple threads (e.g. one motif handled window and another xlib handled window
which is drawn to in another thread but the same xlib display and connect below
it all - as long as you are not consuming events from both threads).

> > it has nothing to do with it. xcb is an artefact of how xlib is implemented
> > today - it absolutely is not part of the spec or design or requirements.
> 
> If my program does not use XCB (or even link with X11-xcb), why should
> it be subject to constraints placed by XCB?  XCB is just one transport
> for the X library.

??? Did you not read what I wrote? You are saying I said the opposite of what I
clearly wrote - xcb has nothing to do with discussing xinitthreads being validly
enabled or not by default. this really has nothing to do with xcb. xlib being
built on xcb is just a happenstance of how xlib is built today and modern code
can actually mix and match xcb and xlib, but it's not part of the core issue.

> > has nothing to do with it. having xlib just be threadsafe always when it is
> > as easy as xinitthreads being the default anyway is nice. it doesn't
> > violate the xlib contract with users of it if they are correctly
> > implemented.
> 
> Nonsense.  The X library has worked how I (and Motif) expect it to since
> at least the early 90s.
> 
> The X11R5 manual page says:
> 
> The XIfEvent function completes only when the specified predicate
> procedure returns  True for an event,  which indicates an event
> in the queue matches.  XIfEvent flushes the output buffer if it
> blocks waiting for additional events.  XIfEvent removes the
> matching event from the queue  and copies the structure into the
> client-supplied XEvent structure.
> 
> When the predicate procedure finds a match, XCheckIfEvent copies
> the matched event into the client-supplied XEvent structure and
> returns  True.  (This event is removed from the queue.)  If the
> predicate procedure finds no match, XCheckIfEvent returns False,
> and the output buffer will have been flushed.  All earlier events
> stored in the queue are not discarded.
> 
> The XPeekIfEvent function returns only when the specified
> predicate procedure returns  True for an event.  After the
> predicate procedure finds a match, XPeekIfEvent copies the
> matched event into the client-supplied XEvent structure without
> removing the event from the queue.  XPeekIfEvent flushes the
> output buffer if it blocks waiting for additional events.
> 
> Notice how there is no mention of calling Xlib functions being unsafe,
> as at that point the X library did not have thread support at all.

And X11R6 added it. various things broke/changed between x11r5 and x11r6.
Certainly parts of ICCCM broke that required minor code changes in WMs and
some clients. the ICCCM has a nice list of them if you want to look it up. They
are not "new features to make use of voluntarily". They are hard "must change
your code" changes. I remember dealing with this back then as there were still
some x11r5 systems floating about that had some subtle differences. You're now
hitting another subtle change only really found now decades later. There was
obvious appetite for making small code breakages back then and this here falls
into that category.



-- 
- Codito, ergo sum - "I code, therefore I am" --
Carsten Haitzler - ras...@rasterman.com