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

[Peter Hutterer]

On Wed, Feb 22, 2017 at 03:06:43PM -0500, Adam Jackson wrote:
> On Wed, 2017-02-22 at 16:50 +0100, Olivier Fourdan wrote:
> > WriteToClient() can be called from XIChangeDeviceProperty() so from the
> > InputThread which is a problem as it allocates and free the input and
> > output buffers.
> > 
> > Add a new mutex to protect accesses to the input and output buffers from
> > being accessed from different threads.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99164
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99887
> > Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> > ---
> >  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.

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. 

Cheers,
   Peter