1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
|
From 8808d16b0bc53e6c1e235d2fc714145b9adcf626 Mon Sep 17 00:00:00 2001
From: Adam Jackson <ajax@redhat.com>
Date: Fri, 5 Aug 2022 15:19:08 -0400
Subject: [PATCH] Allow X*IfEvent() to reenter libX11
The documentation for this family of functions very clearly says not to
call into xlib in your predicate function, but historically single
threaded apps could get away with it just fine, and now that we've
forced thread-safety on the world such apps will now deadlock instead.
That's not an acceptable regression even if the app is technically
broken. This has been reported with XFCE and FVWM, and Motif's
cut-and-paste code has the same bug by inspection, so this does need to
be addressed.
This change nerfs LockDisplay/UnlockDisplay while inside the critical
bit of an IfEvent function. This is still safe in the sense that the
display remains locked and no other thread should be able to change it
from under us, but the loop that scans the event queue might not be
robust against it being modified as a side effect of protocol emitted by
the predicate callback. But that's not new, non-XInitThreads'd xlib
would have the same caveat.
Closes: xorg/lib/libx11#157
---
include/X11/Xlibint.h | 1 +
src/ChkIfEv.c | 3 +++
src/IfEvent.c | 2 ++
src/OpenDis.c | 1 +
src/PeekIfEv.c | 2 ++
src/locking.c | 30 ++++++++++++++++++++++++++++++
6 files changed, 39 insertions(+)
diff --git a/include/X11/Xlibint.h b/include/X11/Xlibint.h
index 6d880a65..46c9ac11 100644
--- a/include/X11/Xlibint.h
+++ b/include/X11/Xlibint.h
@@ -207,6 +207,7 @@ struct _XDisplay
XIOErrorExitHandler exit_handler;
void *exit_handler_data;
+ Bool in_ifevent;
};
#define XAllocIDs(dpy,ids,n) (*(dpy)->idlist_alloc)(dpy,ids,n)
diff --git a/src/ChkIfEv.c b/src/ChkIfEv.c
index 876a850e..327b5eaf 100644
--- a/src/ChkIfEv.c
+++ b/src/ChkIfEv.c
@@ -50,6 +50,7 @@ Bool XCheckIfEvent (
int n; /* time through count */
LockDisplay(dpy);
+ dpy->in_ifevent = True;
prev = NULL;
for (n = 3; --n >= 0;) {
for (qelt = prev ? prev->next : dpy->head;
@@ -60,6 +61,7 @@ Bool XCheckIfEvent (
*event = qelt->event;
_XDeq(dpy, prev, qelt);
_XStoreEventCookie(dpy, event);
+ dpy->in_ifevent = False;
UnlockDisplay(dpy);
return True;
}
@@ -78,6 +80,7 @@ Bool XCheckIfEvent (
/* another thread has snatched this event */
prev = NULL;
}
+ dpy->in_ifevent = False;
UnlockDisplay(dpy);
return False;
}
diff --git a/src/IfEvent.c b/src/IfEvent.c
index ead93dca..a0aed7e3 100644
--- a/src/IfEvent.c
+++ b/src/IfEvent.c
@@ -49,6 +49,7 @@ XIfEvent (
unsigned long qe_serial = 0;
LockDisplay(dpy);
+ dpy->in_ifevent = True;
prev = NULL;
while (1) {
for (qelt = prev ? prev->next : dpy->head;
@@ -59,6 +60,7 @@ XIfEvent (
*event = qelt->event;
_XDeq(dpy, prev, qelt);
_XStoreEventCookie(dpy, event);
+ dpy->in_ifevent = False;
UnlockDisplay(dpy);
return 0;
}
diff --git a/src/OpenDis.c b/src/OpenDis.c
index 5017b040..e1bc2a30 100644
--- a/src/OpenDis.c
+++ b/src/OpenDis.c
@@ -189,6 +189,7 @@ XOpenDisplay (
dpy->xcmisc_opcode = 0;
dpy->xkb_info = NULL;
dpy->exit_handler_data = NULL;
+ dpy->in_ifevent = False;
/*
* Setup other information in this display structure.
diff --git a/src/PeekIfEv.c b/src/PeekIfEv.c
index 207cd119..c4e8af0d 100644
--- a/src/PeekIfEv.c
+++ b/src/PeekIfEv.c
@@ -50,6 +50,7 @@ XPeekIfEvent (
unsigned long qe_serial = 0;
LockDisplay(dpy);
+ dpy->in_ifevent = True;
prev = NULL;
while (1) {
for (qelt = prev ? prev->next : dpy->head;
@@ -63,6 +64,7 @@ XPeekIfEvent (
_XStoreEventCookie(dpy, ©);
*event = copy;
}
+ dpy->in_ifevent = False;
UnlockDisplay(dpy);
return 0;
}
diff --git a/src/locking.c b/src/locking.c
index ea5000e1..36530691 100644
--- a/src/locking.c
+++ b/src/locking.c
@@ -452,6 +452,32 @@ static void _XDisplayLockWait(
}
}
+static void _XLockDisplay(
+ Display *dpy
+ XTHREADS_FILE_LINE_ARGS
+ );
+
+static void _XIfEventLockDisplay(
+ Display *dpy
+ XTHREADS_FILE_LINE_ARGS
+ )
+{
+ /* assert(dpy->in_ifevent); */
+}
+
+static void _XIfEventUnlockDisplay(
+ Display *dpy
+ XTHREADS_FILE_LINE_ARGS
+ )
+{
+ if (dpy->in_ifevent)
+ return;
+
+ dpy->lock_fns->lock_display = _XLockDisplay;
+ dpy->lock_fns->unlock_display = _XUnlockDisplay;
+ UnlockDisplay(dpy);
+}
+
static void _XLockDisplay(
Display *dpy
XTHREADS_FILE_LINE_ARGS
@@ -478,6 +504,10 @@ static void _XLockDisplay(
#endif
_XIDHandler(dpy);
_XSeqSyncFunction(dpy);
+ if (dpy->in_ifevent) {
+ dpy->lock_fns->lock_display = _XIfEventLockDisplay;
+ dpy->lock_fns->unlock_display = _XIfEventUnlockDisplay;
+ }
}
/*
--
GitLab
|
