[PATCH libX11] _XDefaultError: set XlibDisplayIOError flag before calling exit

[Arthur Huillet]

On Wed, 1 Feb 2017 12:23:30 -0800
Jamey Sharp <jamey at minilop.net> wrote:

> On Feb 1, 2017 9:23 AM, "Arthur Huillet" <arthur.huillet at free.fr> wrote:
> 
> On Wed, 1 Feb 2017 09:12:47 -0800
> Jamey Sharp <jamey at minilop.net> wrote:
> > I'm a little confused by the notes in the commit message about whether  
> this
> > is legal. I don't know why it should be considered legal to use a Display
> > from a _fini or atexit handler… although this has been a source of bugs  
> for
> > C++ apps in the past, so clearly people do it. I guess "there exist apps  
> in
> > the wild that do this" is as good a definition of what's legal in Xlib as
> > any. But if you have a more specific reference for why it's legal, it'd be
> > nice to have that.  
> 
> I am surprised that the notion that it is legal would require discussion.
> It's not forbidden anywhere (that I'm aware of), therefore, it's allowed.
> Doesn't that reasoning work?
> 
> 
> I guess that's kind of a philosophical question, made more confusing by
> Xlib's long history.
> 
> In my opinion, and speaking about software in general: no, I don't think
> that's sound reasoning. :-) It leads to brittle software that relies on
> accidents of implementation which may change without notice.
> 
> The story for Xlib is more complicated, because in practice, its
> specification is not its documentation, but rather, "if it worked ten years
> ago, it should probably work now." Of course this means Xlib is nearly
> impossible to maintain. On a related note: many thanks to the people who
> continue maintaining Xlib anyway. :-)
> 
> Yes, it sounds completely useless...
> 
> 
> I never said that. :-) I just thought, from your use of the word "legal",
> that you'd found explicit justification for this behavior in the
> documentation. If you had, then there would be no question that a patch
> like this is necessary, and that would be useful to call out in the commit
> message.

I see. No, I don't have explicit justification.

> 
> As for clarification of the commit message, I thought that the paragraph
> mentioning dlopen/dlclose was covering the problem.
> What do you feel needs clarifying?
> 
> 
> You said all that in the commit message but I didn't understand it. :-)
> It's not super important I guess.
> 
> But: have you considered not using Xlib to issue requests, at least in your
> _fini handler? I think it should always be safe to call XGetXCBConnection
> and then issue your cleanup requests using XCB, which is carefully designed
> to be safe in any context except signal handlers. That would make the
> driver work on older Xlib installations too.

It's something to consider in the future, especially as I have another deadlock-in-Xlib bug, which is harder to fix.
(Very similar scenario, but with threads thrown into the mix for extra fun.)
 
> I'll also point out that I believe using a _fini handler to do X resource
> cleanup is guaranteed to break if the application calls fork. Both the
> parent and the child will attempt to clean up the resources, and one of
> them will find the X connection is in the wrong state. If the other process
> was still using that connection, it will probably hang next time it tries
> to get a reply. Finding a different hook to do cleanup would be better.

This is already a "solved" problem, although I won't get into details because it would give all of us a headache.
See, the GLX and OpenGL specification aren't terribly friendly towards driver developers when it comes to forks,
and we have to detect and support them. The scenario you describe works mostly OK because we already have a bunch
of support code for it. 

> 
> I'll still offer a
> 
> Reviewed-by: Jamey Sharp <jamey at minilop.net>
> 
> because with Xlib, all we can do is harm reduction. If people insist on
> doing things like this, we can at least try to minimize end-users'
> suffering.

Thanks. 

Unrelated, but an upcoming patch of mine will call XInitThreads by default at initialization; unless that is met with strong
opposition. This was discussed years ago, and supported by various people, but never actually done. 

-- 
A. Huillet