| Summary: | Deadlock in _XReply when recursing through _XSeqSyncFunction | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | xorg | Reporter: | Kai Koehne <kai.koehne> | ||||||
| Component: | Lib/Xlib | Assignee: | Xorg Project Team <xorg-team> | ||||||
| Status: | RESOLVED MOVED | QA Contact: | Xorg Project Team <xorg-team> | ||||||
| Severity: | normal | ||||||||
| Priority: | medium | CC: | p.horrocks89, psychon, rdieter, simonandric5, xcb | ||||||
| Version: | unspecified | ||||||||
| Hardware: | x86-64 (AMD64) | ||||||||
| OS: | Linux (All) | ||||||||
| Whiteboard: | |||||||||
| i915 platform: | i915 features: | ||||||||
| Attachments: |
|
||||||||
|
Description
Kai Koehne
2013-01-14 14:30:58 UTC
Main bug report in downstream project (Qt): https://bugreports.qt-project.org/browse/QTBUG-29106 Created attachment 74990 [details] [review] Bugfix? At least this patch makes the test case working here. But, I haven't tested it further and don't know howto. (Except from installing Qt5 stuff.) Do(In reply to comment #2) > Created attachment 74990 [details] [review] [review] > Bugfix? > > At least this patch makes the test case working here. But, I haven't tested > it further and don't know howto. (Except from installing Qt5 stuff.) Doesn't this patch make Xlib lose track of sequence numbers? After all, this GetInputFocus request is meant as a synchronization point to make sure that something close to the current sequence number is known. (However, I don't know much about Xlib internals) It looks like commit 83e1ba59c48c79f8b0a7e7aa0b9c9cfd84fa403d introduced the problem.
The commit unlocks the display when an application-defined error handler is invoked, but re-locking the display requires re-syncing the sequence numbers which requires a remote call.
The commit author couldn't remember why the display lock was held while the error function was called - I guess the sequence number re-sync was the reason.
Reverting the commit fixes the problem.
In the meantime, an application can use this code to work around the problem - it uses a custom extension error handler to cause XError to return before it unlocks/locks the screen:
typedef Bool (*WireToErrorType)(Display*, XErrorEvent*, xError*);
const int NUM_HANDLERS = 256;
WireToErrorType __oldHandlers[NUM_HANDLERS] = {0};
Bool __xErrorHandler(
Display* display,
XErrorEvent* event,
xError* error
)
{
// Call any previous handler first in case it needs to do real work.
auto code = static_cast<int>(event->error_code);
if(__oldHandlers[code] != NULL)
{
__oldHandlers[code](display, event, error);
}
// Always return false so the error does not get passed to the normal
// application defined handler.
return False;
}
void applyXDeadlockFix(Display* display)
{
for(auto i = 0; i < NUM_HANDLERS; ++i)
{
XESetWireToError(display, i, &__xErrorHandler);
}
}
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xorg/lib/libx11/issues/25. |
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.