The current head version of XGetExtensionVersion (XGetVers.c) does not release the display lock when the XInput extension is not enabled on the server. This can easily be tested by running an application that uses libXi over ssh -X with a recent ssh version. When libX11 is compiled with xcb it will fail on the next attempt to obtain a lock. In order to get a better handle on locking in libXi I have created a patch that removes locking related side effects from internal functions (_XiCheckExtInit()) and gets rid of the recursive call from _XiGetExtensionVersion() into _XiCheckExtInit() and back into _XiGetExtensionVersion(). Please review.
Created attachment 7432 [details] [review] Proposed locking cleanup for libXi
Created attachment 7437 [details] [review] Proposed locking cleanup for libXi fixed typo in previous patch
i believe jamey already committed this?
(In reply to comment #3) > i believe jamey already committed this? No, this is a followup of Jamey's work. His fix didn't cover the situation where the extension wasn't present (such as when run over ssh -X)
Sorry I haven't gotten around to this yet. Your patch is a bit overwhelming ;-) but I'll examine it soon.
I've committed a much simpler patch that I believe addresses this bug. Would you inspect/re-test? If it isn't sufficient, please re-open this bug. The clean-ups you've proposed are probably a good idea, but I'm unlikely to be the person to review and apply them.
The patch checked into the libXi git repository works fine for me. Should it also be applied to the 1.0.x branch? This is the one for 7.2, isn't it?
Ah, that seems likely.
Daniel, I've fixed this bug in git master for libXi, and Magnus has verified the fix. Should the fix be cherry-picked for other branches to be included in X.org 7.2? If so, I'll leave that to you. You might also consider some variant of the larger refactoring Magnus proposed, in your copious spare time. I went for a small fix that didn't affect the locking postconditions on _XiCheckExtInit, but those postconditions are stupid ;-) so fixing them wouldn't be a bad thing.
jamey: i don't have any experience in this area, so your insight is certainly more than welcome. we're shipping 1.1 with 7.2, so we should be okay if your patch works?
Sorry about the phenomenal bug spam, guys. Adding xorg-team@ to the QA contact so bugs don't get lost in future.
Shouldn't this bug be resolved? The fix has landed in git.
Move to 7.3 tracker.
already fixed.
Does this also fix bug 5424? (https://bugs.freedesktop.org/show_bug.cgi?id=5424 - Deadlock in XGetExtensionVersion() when XInitThreads() is called before)
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.