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
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
|
mistranslation: 0.838
semantic: 0.788
graphic: 0.768
instruction: 0.767
other: 0.758
assembly: 0.753
device: 0.739
boot: 0.718
vnc: 0.687
KVM: 0.673
socket: 0.660
network: 0.611
Cocoa UI always crashes on startup
Commit 8bb93c6f99a42c2e0943bc904b283cd622d302c5 ("ui/console: ensure graphic updates don't race with TCG vCPUs") causes the graphic update to run on a non-main thread, which Cocoa is not happy with. It crashes immediately after startup.
$ i386-softmmu/qemu-system-i386
2017-03-22 10:09:25.113 qemu-system-i386[15968:9538245] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'nextEventMatchingMask should only be called from the Main Thread!'
*** First throw call stack:
(
0 CoreFoundation 0x00007fff91e72e7b __exceptionPreprocess + 171
1 libobjc.A.dylib 0x00007fffa6a5ccad objc_exception_throw + 48
2 AppKit 0x00007fff900953fd -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 4471
3 qemu-system-i386 0x0000000104f75a49 cocoa_refresh + 233
4 qemu-system-i386 0x0000000104e0312c process_queued_cpu_work + 140
5 qemu-system-i386 0x0000000104d1a73c qemu_tcg_rr_cpu_thread_fn + 284
6 libsystem_pthread.dylib 0x00007fffa7557aab _pthread_body + 180
7 libsystem_pthread.dylib 0x00007fffa75579f7 _pthread_body + 0
8 libsystem_pthread.dylib 0x00007fffa75571fd thread_start + 13
)
libc++abi.dylib: terminating with uncaught exception of type NSException
Abort trap: 6
System: macOS 10.12.3, Xcode 8.2.1
On 22 March 2017 at 17:26, Brendan Shanks <email address hidden> wrote:
> Public bug reported:
>
> Commit 8bb93c6f99a42c2e0943bc904b283cd622d302c5 ("ui/console: ensure
> graphic updates don't race with TCG vCPUs") causes the graphic update to
> run on a non-main thread, which Cocoa is not happy with. It crashes
> immediately after startup.
Oops. Alex, we can't just run UI code on random threads like this.
Any ideas?
thanks
-- PMM
Peter Maydell <email address hidden> writes:
> On 22 March 2017 at 17:26, Brendan Shanks <email address hidden> wrote:
>> Public bug reported:
>>
>> Commit 8bb93c6f99a42c2e0943bc904b283cd622d302c5 ("ui/console: ensure
>> graphic updates don't race with TCG vCPUs") causes the graphic update to
>> run on a non-main thread, which Cocoa is not happy with. It crashes
>> immediately after startup.
>
> Oops. Alex, we can't just run UI code on random threads like this.
Technically its not a random thread its the vCPU context (which ensures
the vCPU isn't updating while the display is being updated). But I guess
the Cocoa is limited to not being able to update from an arbitrary
thread?
There was a patch posted yesterday to ensure the BQL is held during the
deferred work but this doesn't look like that.
> Any ideas?
Hmm a quick Google seems to imply Cocoa is inflexible in its
requirements. You can try this ugly but untested patch (I don't have any
Macs handy):
modified ui/console.c
@@ -1598,8 +1598,16 @@ static void dpy_refresh(DisplayState *s)
QLIST_FOREACH(dcl, &s->listeners, next) {
if (dcl->ops->dpy_refresh) {
if (tcg_enabled()) {
+#ifdef CONFIG_COCOA
+ qemu_mutex_unlock_iothread();
+ start_exclusive();
+ do_safe_dpy_refresh(first_cpu, RUN_ON_CPU_HOST_PTR(dcl));
+ end_exclusive();
+ qemu_mutex_lock_iothread();
+#else
async_safe_run_on_cpu(first_cpu, do_safe_dpy_refresh,
RUN_ON_CPU_HOST_PTR(dcl));
+#endif
} else {
dcl->ops->dpy_refresh(dcl);
}
Other than that I guess we need to bring forward the plans to "fixed the dirty tracking
races in display adapters"
>
> thanks
> -- PMM
--
Alex Bennée
On 23 March 2017 at 11:13, Alex Bennée <email address hidden> wrote:
> Technically its not a random thread its the vCPU context (which ensures
> the vCPU isn't updating while the display is being updated). But I guess
> the Cocoa is limited to not being able to update from an arbitrary
> thread?
Yes. It's very common for windowing libraries to mandate that you
do all windowing operations from one specific thread.
thanks
-- PMM
Peter Maydell <email address hidden> writes:
> On 23 March 2017 at 11:13, Alex Bennée <email address hidden> wrote:
>> Technically its not a random thread its the vCPU context (which ensures
>> the vCPU isn't updating while the display is being updated). But I guess
>> the Cocoa is limited to not being able to update from an arbitrary
>> thread?
>
> Yes. It's very common for windowing libraries to mandate that you
> do all windowing operations from one specific thread.
Fair enough. Well let me know if that works OK on MacOS and I'll fold it
in to the other console patches. In fact I might as well do the
start/end_exclusive dance for all OSes and it will achieve the same thing.
--
Alex Bennée
On Mar 23, 2017, at 7:35 AM, <email address hidden> wrote:
> Message: 15
> Date: Thu, 23 Mar 2017 11:13:02 +0000
> From: Alex Benn?e <email address hidden>
> To: Peter Maydell <email address hidden>
> Cc: Bug 1675108 <email address hidden>, QEMU Developers
> <email address hidden>, Gerd Hoffmann <email address hidden>
> Subject: Re: [Qemu-devel] [Bug 1675108] [NEW] Cocoa UI always crashes
> on startup
> Message-ID: <email address hidden>
> Content-Type: text/plain; charset=utf-8
>
>
> Peter Maydell <email address hidden> writes:
>
>> On 22 March 2017 at 17:26, Brendan Shanks <email address hidden> wrote:
>>> Public bug reported:
>>>
>>> Commit 8bb93c6f99a42c2e0943bc904b283cd622d302c5 ("ui/console: ensure
>>> graphic updates don't race with TCG vCPUs") causes the graphic update to
>>> run on a non-main thread, which Cocoa is not happy with. It crashes
>>> immediately after startup.
>>
>> Oops. Alex, we can't just run UI code on random threads like this.
>
> Technically its not a random thread its the vCPU context (which ensures
> the vCPU isn't updating while the display is being updated). But I guess
> the Cocoa is limited to not being able to update from an arbitrary
> thread?
>
> There was a patch posted yesterday to ensure the BQL is held during the
> deferred work but this doesn't look like that.
>
>> Any ideas?
>
> Hmm a quick Google seems to imply Cocoa is inflexible in its
> requirements. You can try this ugly but untested patch (I don't have any
> Macs handy):
>
> modified ui/console.c
> @@ -1598,8 +1598,16 @@ static void dpy_refresh(DisplayState *s)
> QLIST_FOREACH(dcl, &s->listeners, next) {
> if (dcl->ops->dpy_refresh) {
> if (tcg_enabled()) {
> +#ifdef CONFIG_COCOA
> + qemu_mutex_unlock_iothread();
> + start_exclusive();
> + do_safe_dpy_refresh(first_cpu, RUN_ON_CPU_HOST_PTR(dcl));
> + end_exclusive();
> + qemu_mutex_lock_iothread();
> +#else
> async_safe_run_on_cpu(first_cpu, do_safe_dpy_refresh,
> RUN_ON_CPU_HOST_PTR(dcl));
> +#endif
> } else {
> dcl->ops->dpy_refresh(dcl);
> }
>
>
> Other than that I guess we need to bring forward the plans to "fixed the dirty tracking
> races in display adapters"
>
>>
>> thanks
>> -- PMM
>
>
> --
> Alex Benn?e
Your patch does work. I tested it on Mac OS 10.6.8 using qemu-sytem-ppc.
Has anyone checked on the GTK front-end yet to see if it is having similar problems?
Tested on 10.12.3, it doesn't crash immediately (like before) but crashes reliably once I send some keyboard input to qemu:
$ i386-softmmu/qemu-system-i386
**
ERROR:/Users/pip/no_backup/qemu/translate-common.c:34:tcg_handle_interrupt: assertion failed: (qemu_mutex_iothread_locked())
Abort trap: 6
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0 libsystem_kernel.dylib 0x00007fffa746edd6 __pthread_kill + 10
1 libsystem_pthread.dylib 0x00007fffa755a787 pthread_kill + 90
2 libsystem_c.dylib 0x00007fffa73d4420 abort + 129
3 libglib-2.0.0.dylib 0x00000001076aec86 g_assertion_message + 388
4 libglib-2.0.0.dylib 0x00000001076aece4 g_assertion_message_expr + 94
5 qemu-system-i386 0x00000001066bb1ec tcg_handle_interrupt + 156 (translate-common.c:50)
6 qemu-system-i386 0x0000000106740dfc pic_irq_request + 156 (pc.c:187)
7 qemu-system-i386 0x000000010673e5e4 gsi_handler + 36 (pc.c:115)
8 qemu-system-i386 0x000000010685e97a kbd_update_kbd_irq + 138 (pckbd.c:180)
9 qemu-system-i386 0x000000010694164a qemu_input_event_send_impl + 938 (input.c:328)
10 qemu-system-i386 0x000000010694188f qemu_input_event_send_key + 239 (input.c:359)
11 qemu-system-i386 0x0000000106946a00 cocoa_refresh + 272 (cocoa.m:1402)
12 qemu-system-i386 0x000000010693f6a8 gui_update + 104 (console.c:1603)
The keyboard input issue looks the same as #1675549, and that's on Linux/SDL. So not specific to this fix or Cocoa.
Brendan Shanks <email address hidden> writes:
> Tested on 10.12.3, it doesn't crash immediately (like before) but
> crashes reliably once I send some keyboard input to qemu:
>
> $ i386-softmmu/qemu-system-i386
> **
> ERROR:/Users/pip/no_backup/qemu/translate-common.c:34:tcg_handle_interrupt: assertion failed: (qemu_mutex_iothread_locked())
> Abort trap: 6
Can you test with the suggested patch I posted yesterday? If we keep the
update under BQL this shouldn't fail.
>
>
> Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
> 0 libsystem_kernel.dylib 0x00007fffa746edd6 __pthread_kill + 10
> 1 libsystem_pthread.dylib 0x00007fffa755a787 pthread_kill + 90
> 2 libsystem_c.dylib 0x00007fffa73d4420 abort + 129
> 3 libglib-2.0.0.dylib 0x00000001076aec86 g_assertion_message + 388
> 4 libglib-2.0.0.dylib 0x00000001076aece4 g_assertion_message_expr + 94
> 5 qemu-system-i386 0x00000001066bb1ec tcg_handle_interrupt + 156 (translate-common.c:50)
> 6 qemu-system-i386 0x0000000106740dfc pic_irq_request + 156 (pc.c:187)
> 7 qemu-system-i386 0x000000010673e5e4 gsi_handler + 36 (pc.c:115)
> 8 qemu-system-i386 0x000000010685e97a kbd_update_kbd_irq + 138 (pckbd.c:180)
> 9 qemu-system-i386 0x000000010694164a qemu_input_event_send_impl + 938 (input.c:328)
> 10 qemu-system-i386 0x000000010694188f qemu_input_event_send_key + 239 (input.c:359)
> 11 qemu-system-i386 0x0000000106946a00 cocoa_refresh + 272 (cocoa.m:1402)
> 12 qemu-system-i386 0x000000010693f6a8 gui_update + 104 (console.c:1603)
--
Alex Bennée
On Do, 2017-03-23 at 11:31 +0000, Alex Bennée wrote:
> Peter Maydell <email address hidden> writes:
>
> > On 23 March 2017 at 11:13, Alex Bennée <email address hidden> wrote:
> >> Technically its not a random thread its the vCPU context (which ensures
> >> the vCPU isn't updating while the display is being updated). But I guess
> >> the Cocoa is limited to not being able to update from an arbitrary
> >> thread?
> >
> > Yes. It's very common for windowing libraries to mandate that you
> > do all windowing operations from one specific thread.
>
> Fair enough. Well let me know if that works OK on MacOS and I'll fold it
> in to the other console patches. In fact I might as well do the
> start/end_exclusive dance for all OSes and it will achieve the same thing.
Where do we stand with this one?
cheers,
Gerd
Gerd Hoffmann <email address hidden> writes:
> On Do, 2017-03-23 at 11:31 +0000, Alex Bennée wrote:
>> Peter Maydell <email address hidden> writes:
>>
>> > On 23 March 2017 at 11:13, Alex Bennée <email address hidden> wrote:
>> >> Technically its not a random thread its the vCPU context (which ensures
>> >> the vCPU isn't updating while the display is being updated). But I guess
>> >> the Cocoa is limited to not being able to update from an arbitrary
>> >> thread?
>> >
>> > Yes. It's very common for windowing libraries to mandate that you
>> > do all windowing operations from one specific thread.
>>
>> Fair enough. Well let me know if that works OK on MacOS and I'll fold it
>> in to the other console patches. In fact I might as well do the
>> start/end_exclusive dance for all OSes and it will achieve the same thing.
>
> Where do we stand with this one?
I've got two patches in my tree at the moment. I was holding off posting
the series to see if I could make more progress with the record/replay
bug. I'll post the series tomorrow morning but if you want to grab them
ahead of that see:
https://github.com/stsquad/qemu/commit/6c0ddfc5752f311b4c5a2956de25821cc2cd3fd6
https://github.com/stsquad/qemu/commit/15d2b05a20879017f20370b71d5d144947b693fe
--
Alex Bennée
On 27 March 2017 at 16:18, Alex Bennée <email address hidden> wrote:
> I've got two patches in my tree at the moment. I was holding off posting
> the series to see if I could make more progress with the record/replay
> bug.
rc candidates are cut on Tuesdays, so it's better in general not
to sit on a fix for rc bugs if it causes them to miss going into
the next rc tag.
thanks
-- PMM
I just did a quick test on 10.12.3 with those two patches and didn't get any crashes
Brendan Shanks <email address hidden> writes:
> I just did a quick test on 10.12.3 with those two patches and didn't get
> any crashes
Awesome. I'm rolling the series now. I assume will pickup the patches in
due course.
--
Alex Bennée
Fixed in -rc2, closing.
|