Re: XInitThreads in library constructor breaks Motif!
On Sun, Oct 30, 2022 at 04:08:50PM -0700, Alan Coopersmith wrote:
> On 10/29/22 19:00, Po Lu wrote:
> > Motif assumes that you can call functions that lock the display inside
> > XCheckIfEvent as it never calls XInitThreads itself. It also never uses
> > threads.
>
> It was never up to Motif to decide whether or not to call XInitThreads()
> - that decision was up to the applications, and if Motif assumed no
> application would ever do that, it was buggy and incompatible with such
> applications.
>
> https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/150 is
> a proposed solution to the problem, but has not generated any interest
> in testing it or moving it forward, suggesting either everyone is
> building with the --disable-thread-safety-constructor option or is
> using correctly written software that does not make such assumptions.
>
Oh I did miss this MR. It looks like a step in the proper direction,
since after trying to patch X*IfEvent() callbacks in several places, I
see how painful it can be. And in OpenBSD I also had to eventually add
--disable-thread-safety-constructor to the build of libX11 1.8.1.
However one of the software that's doing a lot of Xlib calls in the
X*IfEvents() callbacks is fvwm{2,3}. The MR I send them is wrong, and I
fear they are fiddling with the input queue in the callbacks. I'll
have to test how it behaves with this change.
--
Matthieu Herrb
Re: XInitThreads in library constructor breaks Motif!
On Mon, 31 Oct 2022 08:11:09 +0800 Po Lu said: > > > On October 31, 2022 7:08:50 AM GMT+08:00, Alan Coopersmith > wrote: > >On 10/29/22 19:00, Po Lu wrote: > >> Motif assumes that you can call functions that lock the display inside > >> XCheckIfEvent as it never calls XInitThreads itself. It also never uses > >> threads. > > > >It was never up to Motif to decide whether or not to call XInitThreads() > >- that decision was up to the applications, and if Motif assumed no > >application would ever do that, it was buggy and incompatible with such > >applications. > > Okay, so what about the countless Motif *applications* that don't use > threads? Motif is not a thread-safe library, and is documented as such, so > Motif applications (mine included) deliberately do not call XInitThreads. using motif and using xlib are different things. you are still free to use xlib separately to motif and thus use it from multiple threads... as long as you use motif and only motif from a single thread, you agree to the non-threadsafe contract. > >https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/150 is > >a proposed solution to the problem, but has not generated any interest > >in testing it or moving it forward, suggesting either everyone is > >building with the --disable-thread-safety-constructor option or is > >using correctly written software that does not make such assumptions. > > Or most people are on Xlib 1.7.x. Anyway, the documentation for X*IfEvent > says it is okay to call functions that take the display lock inside the > predicate given that it does not change the event queue. I have written such > software, and clearly the Open Group has too, along with XFCE, etc. the solution to this is... use a recursive lock in xlib or change the locking around xcheckifevent and friends to unlock when going into the callback (which is the current proposed solution in gitlab). the point of locking is to keep xlib working with multiple threads as it would with a single one and this would honor that goal :) > Besides, how to move it forward? If it works, wouldn't the obvious solution > be to install the change? If all that is needed is for someone to try it out, > I'd be happy to do it, but I'm pretty sure the better solution is just to > remove the call to XInitThreads in the first place. > > So please don't make arbitrary changes to documented behavior!! There are > people who pay for Motif, and such things did not happen when both Motif and > Xlib were being developed under the auspices of the Open Group. > > Thanks. > -- - Codito, ergo sum - "I code, therefore I am" -- Carsten Haitzler - ras...@rasterman.com
Re: XInitThreads in library constructor breaks Motif!
Keith Packard writes: > As currently described, the fix is untested. I'd encourage you to apply > it and see if it works; that would at least provide some evidence that > it helps. It helps for Motif programs. But what about other X extensions that may want to run callbacks inside LockDisplay?
Re: XInitThreads in library constructor breaks Motif!
Carsten Haitzler writes: > using motif and using xlib are different things. you are still free to use > xlib > separately to motif and thus use it from multiple threads... as long as you > use > motif and only motif from a single thread, you agree to the non-threadsafe > contract. Which Xlib breaks, by initializing threads. > the solution to this is... use a recursive lock in xlib or change the > locking around xcheckifevent and friends to unlock when going into the > callback (which is the current proposed solution in gitlab). the point of > locking is to keep xlib working with multiple threads as it would with a > single > one and this would honor that goal :) XCB afaik doesn't start multiple threads itself, so I don't see the problem at all. If there is really a big problem with clients using XCB, then why not call XInitThreads in XGetXCBConnection instead?
Re: XInitThreads in library constructor breaks Motif!
On Mon, Oct 31, 2022 at 8:18 AM Po Lu wrote: If there is really a big problem with clients using XCB, then why not > call XInitThreads in XGetXCBConnection instead? > Because XInitThreads only works if it is called before XOpenDisplay. - ajax
Fwd: XInitThreads in library constructor breaks Motif!
Bah, meant to reply all here. - ajax -- Forwarded message - From: Adam Jackson Date: Mon, Oct 31, 2022 at 12:29 PM Subject: Re: XInitThreads in library constructor breaks Motif! To: Po Lu On Sun, Oct 30, 2022 at 8:10 PM Po Lu wrote: > > Or most people are on Xlib 1.7.x. Anyway, the documentation for X*IfEvent > says it is okay to call functions that take the display lock inside the > predicate given that it does not change the event queue. I have written > such software, and clearly the Open Group has too, along with XFCE, etc. > The documentation absolutely does not say that. The manual page says "Your predicate procedure must decide if the event is useful without calling any Xlib functions". Taking the display lock is an xlib function. It also clearly _calls out_ a case where behavior will _definitely_ be undefined, but the "do not re-enter xlib" restriction is not limited to "do not use xlib to touch the event queue". Besides, how to move it forward? If it works, wouldn't the obvious solution > be to install the change? If all that is needed is for someone to try it > out, I'd be happy to do it, but I'm pretty sure the better solution is just > to remove the call to XInitThreads in the first place. > No. xlib needs to be thread safe in order for other libraries and apps to use xcb reliably. The socket handoff mechanism between xcb and xlib _cannot work safely_ unless the xlib display is thread safe. It will only be thread-safe if XInitThreads was called before XOpenDisplay. The xcb-using component cannot possibly enforce that because the xcb connection is created by xlib, so (for example) libGL would be left with no other option than to refuse to work on non-thread-safe displays. - ajax
Re: XInitThreads in library constructor breaks Motif!
On Mon, 31 Oct 2022 20:18:02 +0800 Po Lu said: > Carsten Haitzler writes: > > > using motif and using xlib are different things. you are still free to use > > xlib separately to motif and thus use it from multiple threads... as long > > as you use motif and only motif from a single thread, you agree to the > > non-threadsafe contract. > > Which Xlib breaks, by initializing threads. 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. > > the solution to this is... use a recursive lock in xlib or change the > > locking around xcheckifevent and friends to unlock when going into the > > callback (which is the current proposed solution in gitlab). the point of > > locking is to keep xlib working with multiple threads as it would with a > > single one and this would honor that goal :) > > XCB afaik doesn't start multiple threads itself, so I don't see the > problem at all. 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 there is really a big problem with clients using XCB, then why not > call XInitThreads in XGetXCBConnection instead? 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. as some have not been - the proposed patch up in the thread works around that. another possible solution would be a recursive lock. -- - Codito, ergo sum - "I code, therefore I am" -- Carsten Haitzler - ras...@rasterman.com
Re: XInitThreads in library constructor breaks Motif!
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.
Re: XInitThreads in library constructor breaks Motif!
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: > 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. > 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.
