[PATCH RFC xserver] os: Add a mutex to protect io buffers

[Olivier Fourdan]

Hi Peter,

> > > ---
> > >  RFC: This is probably sub-optimal and broken in many places, but it
> > >       seems to avoid the memory corruption (so far)... At least it's a
> > >       start, I guess.
> > 
> > It's certainly an improvement in correctness, I just hate it. ;)
> > 
> > One problem with this is there are some requests (GetImage in
> > particular) whose reply is split up into multiple calls to
> > WriteToClient, which means if an XICDP call happens on the input thread
> > while GetImage is writing, the events will be interleaved with the
> > reply data. In addition to corrupting the returned image, the client
> > will almost certainly choke and die while attempting to process the
> > next reply/event.
> > 
> > We can paper over that by fixing the WriteToClient callers to
> > (recursively) take the io lock as well. But doing so highlights a
> > design issue with this approach. GetImage replies are slow because
> > they're a lot of data, so now (if you hit the same XICDP path) you've
> > made the input thread _block_ until the request completes, and the
> > mouse will get stuck.
> > 
> > What I'd like to see is the events built asynchronously and flushed
> > from the main thread (possibly only if we can tell we're running on the
> > input thread).
> 
> unless I'm confused about the thread, it's intention was to replace the
> sigio handler. So while it updates the pointer, it never actually *sends*
> events, it merely generates them and shoves them into the EQ. The events are
> then taken from there in the main thread and processed.

>From the backtraces I got, it does send them, see that backtrace I captured yesterday:

#0  0x0000000006f8b91f in __GI_raise (sig=sig at entry=6) at ../sysdeps/unix/sysv/linux/raise.c:58
#1  0x0000000006f8d51a in __GI_abort () at abort.c:89
#2  0x00000000005a156e in OsAbort () at utils.c:1355
#3  0x00000000005a7163 in AbortServer () at log.c:877
#4  0x00000000005a7f4d in FatalError (f=f at entry=0xaaf8ecf "%s:%d assertion '%s' failed\n") at log.c:1015
#5  0x000000000a974e36 in _kgem_submit (kgem=kgem at entry=0x9617000) at kgem.c:4134
#6  0x000000000a9b9f64 in kgem_submit (kgem=0x9617000) at kgem.h:382
#7  0x000000000a9b9f64 in sna_accel_flush (sna=sna at entry=0x850800 <FlushCallback>) at sna_accel.c:17375
#8  0x000000000a9b9fec in sna_flush_callback (list=<optimized out>, user_data=0x850800 <FlushCallback>, call_data=<optimized out>) at sna_accel.c:17399
#9  0x000000000043c3e4 in _CallCallbacks (pcbl=0x36dc6b30, pcbl at entry=0x850800 <FlushCallback>, call_data=0x20, call_data at entry=0xb0bc150) at dixutils.c:737
#10 0x000000000059dbd3 in CallCallbacks (call_data=0xb0bc150, pcbl=0x850800 <FlushCallback>) at ../include/callback.h:83
#11 0x000000000059dbd3 in FlushClient (who=who at entry=0xb0bc150, oc=oc at entry=0x9552800, __extraBuf=__extraBuf at entry=0x11027a60, extraCount=extraCount at entry=32) at io.c:809
#12 0x000000000059e13f in WriteToClient (who=who at entry=0xb0bc150, count=count at entry=32, __buf=__buf at entry=0x11027a60) at io.c:763
#13 0x0000000000442572 in WriteEventsToClient (pClient=pClient at entry=0xb0bc150, count=<optimized out>, count at entry=1, events=events at entry=0x11027a60) at events.c:6000
#14 0x0000000000442710 in TryClientEvents (client=0xb0bc150, dev=<optimized out>, pEvents=0x11027a60, count=1, mask=<optimized out>, filter=<optimized out>, grab=0x0)
    at events.c:2021
#15 0x0000000000445e7a in DeliverEventToInputClients (dev=dev at entry=0xfc85870, inputclients=0xd5726f0, win=win at entry=0xb2fed40, events=events at entry=0x11027a60, count=count at entry=1, filter=filter at entry=16, grab=0x0, client_return=0x11027998, mask_return=0x11027994) at events.c:2170
#16 0x0000000000446167 in DeliverEventToWindowMask (mask_return=0x11027994, client_return=0x11027998, grab=0x0, filter=16, count=1, events=0x11027a60, win=0xb2fed40, dev=0xfc85870) at events.c:2213
#17 0x0000000000446167 in DeliverEventsToWindow (pDev=pDev at entry=0xfc85870, pWin=pWin at entry=0xb2fed40, pEvents=pEvents at entry=0x11027a60, count=count at entry=1, filter=filter at entry=16, grab=grab at entry=0x0) at events.c:2277
#18 0x00000000005283b9 in SendEventToAllWindows (dev=dev at entry=0xfc85870, mask=16, ev=ev at entry=0x11027a60, count=count at entry=1) at exevents.c:3045
#19 0x0000000000533d1f in send_property_event (dev=dev at entry=0xfc85870, property=<optimized out>, what=<optimized out>) at xiproperty.c:208
#20 0x00000000005346d8 in XIChangeDeviceProperty (dev=0xfc85870, property=<optimized out>, type=<optimized out>, format=<optimized out>, mode=<optimized out>, len=<optimized out>, value=0x11027b50, sendevent=1) at xiproperty.c:802
#21 0x000000000fe1d039 in wcmUpdateSerial () at /usr/lib64/xorg/modules/input/wacom_drv.so
#22 0x000000000fe131c2 in wcmSendEvents () at /usr/lib64/xorg/modules/input/wacom_drv.so
#23 0x000000000fe14764 in wcmEvent () at /usr/lib64/xorg/modules/input/wacom_drv.so
#24 0x000000000fe1a476 in usbParse () at /usr/lib64/xorg/modules/input/wacom_drv.so
#25 0x000000000fe11feb in wcmReadPacket () at /usr/lib64/xorg/modules/input/wacom_drv.so
#26 0x000000000fe12226 in wcmDevReadInput () at /usr/lib64/xorg/modules/input/wacom_drv.so
#27 0x000000000059cc8c in InputReady (fd=32, xevents=1, data=0xfc94e90) at inputthread.c:173
#28 0x000000000059f271 in ospoll_wait (ospoll=0xd1172f0, timeout=timeout at entry=-1) at ospoll.c:412
#29 0x000000000059cae6 in InputThreadDoWork (arg=<optimized out>) at inputthread.c:360
#30 0x0000000006d3f6ca in start_thread (arg=0x11029700) at pthread_create.c:333
#31 0x000000000705df7f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:105

> So I'm wondering if the simple fix for all these bugs would be an
> xorg_backtrace() + return in WriteToClient whenever it's called from within
> the input thread.

But then what would happen with the send_property_event() from the XIChangeDeviceProperty() in this case?

Cheers,
Olivier