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 <[email protected]> > --- > 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). - ajax _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
