[Xcb] [PATCH v2 2/2] Also signal special event waiters in _xcb_in_wake_up_next_reader
Michel Dänzer
michel at daenzer.net
Wed Jun 10 03:00:59 PDT 2015
On 10.06.2015 16:32, Uli Schlachter wrote:
> Am 01.06.2015 um 04:09 schrieb Michel Dänzer:
>> On 30.05.2015 21:22, Uli Schlachter wrote:
>>> Am 25.05.2015 um 10:52 schrieb Michel Dänzer:
>>>> From: Michel Dänzer <michel.daenzer at amd.com>
>>>>
>>>> Fixes occasional deadlock when quitting the Mesa demo glthreads with
>>>> DRI3/Present.
>>>>
>>>> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
>>>> ---
>>>>
>>>> v2: Fix indentation of closing curly brace.
>>>>
>>>> src/xcb_in.c | 7 +++++++
>>>> 1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/src/xcb_in.c b/src/xcb_in.c
>>>> index 322bed8..9a8cae1 100644
>>>> --- a/src/xcb_in.c
>>>> +++ b/src/xcb_in.c
>>>> @@ -890,7 +890,14 @@ void _xcb_in_wake_up_next_reader(xcb_connection_t *c)
>>>> if(c->in.readers)
>>>> pthreadret = pthread_cond_signal(c->in.readers->data);
>>>> else
>>>> + {
>>>> + xcb_special_event_t *se;
>>>> +
>>>> pthreadret = pthread_cond_signal(&c->in.event_cond);
>>>> + for (se = c->in.special_events; se; se = se->next) {
>>>> + pthreadret = pthread_cond_signal(&se->special_event_cond);
>
> btw: This should be
>
> pthreadret |= ...
Thanks, updated locally.
>>>> + }
>>>> + }
>>>> assert(pthreadret == 0);
>>>> }
>>>
>>> Hrm. This is basically a pthread_cond_broadcast() waking up all threads, even
>>> those for which we do not have a special event.
>>>
>>> Can you explain why this is necessary at all? The function event_special()
>>> should be handling this. This is called on all special events. It appends the
>>> event to the xcb_special_event_t's event queue and then calls
>>> pthread_cond_signal() on exactly this condition variable.
>>>
>>> In my understanding, that should be enough. Why would we need a "wake up all
>>> waiting threads and let them figure out if they have to do something themselves"?
>>
>> I can't explain why, but what you're saying applies to "normal" events
>> as well, doesn't it? _xcb_in_wake_up_next_reader() signals
>> c->in.event_cond, even though read_packet() already does the same. This
>> change is mirroring that for special events.
>
> Can you try changing your above pthread_cond_signal() into a
> pthread_cond_broadcast?
Not that in contrast to what you wrote above, the code I'm adding isn't
the same as pthread_cond_broadcast: My code calls pthread_cond_signal
for each special event condition variable, waking up one random thread
(or none) waiting for each of them. pthread_cond_broadcast wakes up all
threads waiting for a condition variable.
So, I'm confused why you're now asking me to try waking up even more
threads, when you wrote above that waking any additional threads
shouldn't be necessary.
> Alternatively, change the pthread_cond_signal() in event_special()
> to pthread_cond_broadcast() and see if that helps.
>
> (The idea being: There are multiple threads in xcb_wait_for_special_event()
> concurrently.)
That doesn't fix the first problem described below.
> I tried the reproducing program from [1] and while I can also reproduce the hang
> that your first patch fixes (and I can confirm that the patch fixes the
> problem), I cannot reproduce any hang that this patch would have to fix.
As described in the commit log, the test case for this patch is a
different one: The Mesa demos program called glthreads, which creates
several threads rendering with OpenGL (to one separate window each)
concurrently.
The problem fixed by this patch is that glthreads often hangs when
trying to quit by pressing the Escape key.
Note that glthreads also exhibits another problem, which none of the
changes above fix: Sometimes when starting it, the windows stay black
until I press any key. I'm including the backtrace corresponding to this
hang at the end of this message.
Another problem which I suspect might be related, but which isn't fixed
by these changes, is https://bugs.freedesktop.org/show_bug.cgi?id=90798
All of these problems only occur when the OpenGL implementation uses
the DRI3/Present extensions, which rely on special events.
Thread 2 (Thread 0x7ff7ba9ce700 (LWP 5359)):
#0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1 0x00007ff7c36ab3d5 in _XReply () from /usr/lib/x86_64-linux-gnu/libX11.so.6
#2 0x00007ff7c36a6cad in XSync () from /usr/lib/x86_64-linux-gnu/libX11.so.6
#3 0x00007ff7c3d089bb in dri3_update_drawable (driDrawable=<optimized out>, loaderPrivate=0x7ff7ac0008c0) at ../../../src/glx/dri3_glx.c:1072
#4 dri3_get_buffers (driDrawable=<optimized out>, format=4098, stamp=<optimized out>, loaderPrivate=0x7ff7ac0008c0, buffer_mask=1, buffers=0x7ff7ba9cdb70) at ../../../src/glx/dri3_glx.c:1438
#5 0x00007ff7be9d7d60 in dri_image_drawable_get_buffers (statts_count=<optimized out>, statts=<optimized out>, images=<optimized out>, drawable=<optimized out>) at ../../../../../src/gallium/state_trackers/dri/dri2.c:279
#6 dri2_allocate_textures (ctx=0x10b8270, drawable=0x7ff7ac0009f0, statts=0x7ff7ac001048, statts_count=2) at ../../../../../src/gallium/state_trackers/dri/dri2.c:402
#7 0x00007ff7be9d3dbd in dri_st_framebuffer_validate (stctx=<optimized out>, stfbi=<optimized out>, statts=0x7ff7ac001048, count=2, out=0x7ff7ba9cdcd0) at ../../../../../src/gallium/state_trackers/dri/dri_drawable.c:83
#8 0x00007ff7be8bdfde in st_framebuffer_validate (stfb=0x7ff7ac000c00, st=st at entry=0x111cfb0) at ../../../src/mesa/state_tracker/st_manager.c:200
#9 0x00007ff7be8bf513 in st_api_make_current (stapi=<optimized out>, stctxi=0x111cfb0, stdrawi=0x7ff7ac0009f0, streadi=0x7ff7ac0009f0) at ../../../src/mesa/state_tracker/st_manager.c:769
#10 0x00007ff7be9d38f1 in dri_make_current (cPriv=<optimized out>, driDrawPriv=0x7ff7ac0009b0, driReadPriv=0x7ff7ac0009b0) at ../../../../../src/gallium/state_trackers/dri/dri_context.c:234
#11 0x00007ff7be9cf314 in driBindContext (pcp=<optimized out>, pdp=<optimized out>, prp=<optimized out>) at ../../../../../../src/mesa/drivers/dri/common/dri_util.c:538
#12 0x00007ff7c3d063da in dri3_bind_context (context=0x10b8090, old=<optimized out>, draw=<optimized out>, read=<optimized out>) at ../../../src/glx/dri3_glx.c:145
#13 0x00007ff7c3cd948e in MakeContextCurrent (gc_user=0x10b8090, read=39845893, draw=39845893, dpy=0xfd9c80) at ../../../src/glx/glxcurrent.c:228
#14 glXMakeCurrent (dpy=0xfd9c80, draw=39845893, gc=0x10b8090) at ../../../src/glx/glxcurrent.c:262
#15 0x0000000000402588 in draw_loop (wt=0x6045b8 <WinThreads+56>) at glthreads.c:243
#16 thread_function (p=0x6045b8 <WinThreads+56>) at glthreads.c:526
#17 0x00007ff7c32380a4 in start_thread (arg=0x7ff7ba9ce700) at pthread_create.c:309
#18 0x00007ff7c2f6604d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
Thread 1 (Thread 0x7ff7c46377c0 (LWP 5356)):
#0 0x00007ff7c2f5d50d in poll () at ../sysdeps/unix/syscall-template.S:81
#1 0x00007ff7c0e3bfd2 in _xcb_conn_wait (c=0xfdb3d0, cond=<optimized out>, vector=0x0, count=0x0) at ../../src/xcb_conn.c:479
#2 0x00007ff7c0e3db6f in xcb_wait_for_event (c=0x7ffc7b991740) at ../../src/xcb_in.c:668
#3 0x00007ff7c36aafe8 in _XReadEvents () from /usr/lib/x86_64-linux-gnu/libX11.so.6
#4 0x00007ff7c369a078 in XNextEvent () from /usr/lib/x86_64-linux-gnu/libX11.so.6
#5 0x0000000000401ff5 in event_loop (dpy=<optimized out>) at glthreads.c:378
#6 main (argc=<optimized out>, argv=<optimized out>) at glthreads.c:684
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
More information about the Xcb
mailing list