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

[Jamey Sharp]

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.

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.

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.

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.

Jamey
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.x.org/archives/xorg-devel/attachments/20170201/23fc0670/attachment.html>