[PATCH libX11] xcb_io: Fix Xlib 32-bit request number issues

Christian Linhart chris at DemoRecorder.com
Tue Nov 4 06:45:00 PST 2014 

Hi Jan and Jonas,

I have quickly skimmed your patch.

I may not have found all issues but I have already two comments:

* I think we should have xcb expose 64-bit sequence numbers in its API.

  This can be probably done in a backward compatible way by introducing new functions.
  The old functions can then be wrappers around the new functions.
  This needs to be looked at in more detail however.

  That way, we'll need less workaround-style code in xcb-io.c 
  and the remaining patch will be simpler.
  Therefore more likely to be correct, and easier to understand.

* We still have the unimplemented case
     throw_thread_fail_assert("Sequence number wrapped "
  in xcb_io.c line 81.

  This can probably also be resolved if we expose 64-bit sequence numbers in the XCB-API.


I'll need to look into more details of all of this.

So far, does anyone have any comments to my comments?

Chris

P.S:
I also have a mission-critical software that must not crash under any circumstances.

I still use old non-XCB libX11 from X11R6.9 with some modifications
in the most mission-critical component of my application.
So this component will not be affected.

But some not-so-mission-critical components may be affected.
And that's not nice either.

So this has rather high priority for me.
I'll probably make a new patch which fixes those issues, by making
xcb expose 64-bit sequence numbers.
This may take some time however because this is a very critical piece of code,
so I have to think it through very carefully.

Until this is fixed with an official version, I suggest that you either 
* statically link versions of libX11 and libxcb which work reliably with your application to your application, 
* or that you ship shared libs which you redirect to by setting LD_LIBRARY_PATH accordingly in the 
  startup-scripts of your application.


P.P.S.:
The 16-bit sequence-number issue which I have mentioned before is a different issue.
I have to look into that in more depth, too.


On 11/04/14 14:24, Jan Smout wrote:
> Hi Christian,
> 
> thank you for having a look. The original patch is from Jonas Petersen. You will find all necessary information in the links below:
>    orginal:https://freedesktop.org/patch/16753/
>    latest:  https://freedesktop.org/patch/33513/
> 
> I got involved just because I happen to have a critical application which is not allowed to crash, let alone after less than 24 hrs:
> http://lists.x.org/archives/xorg-devel/2014-August/043661.html
> 
> 
> other references:
> https://bugs.freedesktop.org/show_bug.cgi?id=71338
> http://lists.x.org/archives/xorg-devel/2013-October/038370.html
> https://freedesktop.org/patch/15500/
> 
> best regards,
> Jan
> 
> On 4 November 2014 13:55, Christian Linhart <chris at demorecorder.com <mailto:chris at demorecorder.com>> wrote:
> 
>     Hi Jan,
> 
>     Can you please repost your patch together with a description of the problem and your approach to fix it.
> 
>     I was not subscribed to that list back when you have posted it, and so may some others, who may be able to move this thing forward. You may also post a link to the specific mails/threads in the mailinglist-archives if that'll help with understanding your patch.
> 
>     I'll look at that then.
> 
>     I have some other issues with sequence numbers, and I think we need to solve that on a design level.
>     The essential thing is that the protocol transports only 16-bit sequence numbers, but server and client work with at least 32-bit, sometimes 64-bit sequence numbers. What I've seen so far in the code looks like heuristics which may fail in some cases. Maybe I am missing something. In any case, I want to see your problem analysis and your proposed fix before I propose something.
> 
>     Chris
> 
> 
>     On 11/04/14 13:14, Jan Smout wrote:
>>
>>     I already forked it for myself a couple of months ago. As long as I control the packages which get installed on the machines I have no real issue... except for an uncomfortable feeling that if things like this don't get fixed, what other dragons might be hiding deep down in the xlib library?
>>     Now, when somebody would want to run the application on their own install, that's where the shit hits the fan. I'll be forced to tell them to downgrade their xlib to 1.3.3 and file a complaint on this list :-)
>>
>>
>>     On 4 November 2014 10:49, Alexander E. Patrakov <patrakov at gmail.com <mailto:patrakov at gmail.com>> wrote:
>>
>>         Just fork it. I am sure that such antisocial step is the only way forward, because I also have a patch that was not looked at for too long, and then rejected because it breaks keystone correction (which was broken in a different way before the patch).
>>
>>
>>         04.11.2014 13:23, Jan Smout wrote:
>>>         and again... reminder...
>>>
>>>         On 28 October 2014 12:51, Jan Smout <smout.jan at gmail.com <mailto:smout.jan at gmail.com>> wrote:
>>>
>>>             reminder...
>>>
>>>             On 21 October 2014 12:49, Jan Smout <smout.jan at gmail.com <mailto:smout.jan at gmail.com>> wrote:
>>>
>>>
>>>                 Keith, we are approaching the one year anniversary of this bug already. Maybe it is time to finish the patch and leave the issue behind?
>>>
>>>
>>>                 fyi, I have been running my application with the first version of Jonas's patch for 65 days straight now without a glitch (it used to crash in less than 20 hours).
>>>
>>>                 I also intend to restart this long duration test once the final patch will be released
>>>
>>>
>>>                 On 27 September 2014 05:23, Keith Packard <keithp at keithp.com <mailto:keithp at keithp.com>> wrote:
>>>
>>>                     Matt Turner <mattst88 at gmail.com <mailto:mattst88 at gmail.com>> writes:
>>>
>>>                     > On Fri, Sep 26, 2014 at 3:40 AM, Jan Smout <smout.jan at gmail.com <mailto:smout.jan at gmail.com>> wrote:
>>>                     >> Keith Packard doesn't seem very responsive (as in 'completely ignoring the
>>>                     >> subject')
>>>                     >
>>>                     > Perhaps you should try Ccing him? (now Cc'd)
>>>
>>>                     The problem is that reviewing this patch is *really hard*. The last
>>>                     time, I think I spent a solid couple of days thinking about this and
>>>                     making sure I'd caught all of the cases. I'm still not sure it's right,
>>>                     but I guess it's probably better than what we have?
>>>
>>>                     --
>>>                     keith.packard at intel.com <mailto:keith.packard at intel.com>
>>>
>>>
>>>
>>>
>>>                 -- 
>>>                 Life is complex, it has a real part and an imaginary part.
>>>
>>>
>>>
>>>
>>>             -- 
>>>             Life is complex, it has a real part and an imaginary part.
>>>
>>>
>>>
>>>
>>>         -- 
>>>         Life is complex, it has a real part and an imaginary part.
>>>
>>>
>>>         _______________________________________________
>>>         xorg-devel at lists.x.org <mailto:xorg-devel at lists.x.org>: X.Org development
>>>         Archives: http://lists.x.org/archives/xorg-devel
>>>         Info: http://lists.x.org/mailman/listinfo/xorg-devel
>>
>>
>>
>>
>>     -- 
>>     Life is complex, it has a real part and an imaginary part.
>>
>>
>>     _______________________________________________
>>     xorg-devel at lists.x.org: <mailto:xorg-devel at lists.x.org:> X.Org development
>>     Archives: http://lists.x.org/archives/xorg-devel
>>     Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 
> 
> 
> 
> -- 
> Life is complex, it has a real part and an imaginary part.
> 
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel

More information about the xorg-devel mailing list