New issue
Advanced search Search tips

Issue 870884 link

Starred by 1 user
Status: Untriaged
Owner: ----
Cc:
thestig@chromium.org
thomasanderson@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

Can there be re-entrancy in X11 event dispatch?

Project Member Reported by erikc...@chromium.org, Aug 3 
Most other operating systems do have re-entrancy if something [chrome or system library] spins a nested run loop.

I notice that the setter of dispatching_event_:
https://cs.chromium.org/chromium/src/ui/events/platform/x11/x11_event_source.cc?g=0&l=234

is not re-entrant safe. This is used to compute X11EventSource::GetTimestamp(), which also makes the assumption that there's only a single event being dispatched.

Comment 1 by thomasanderson@chromium.org, Aug 3 

I don't think there's any way to re-enter X11EventSource::ExtractCookieDataDispatchEvent().  IIRC, we push a new X11EventSource for nested message loops.

X11 is an asynchronous protocol, and X11EventSource::GetTimestamp() is naughty and takes advantage of this by processing events out-of-order.  It's true that there may be multiple requests we've sent to the server which we're still waiting for responses on.

Comment 2 by erikc...@chromium.org, Aug 3 

err, X11EventSource is a singleton -- see X11EventSource::GetInstance. And this gets used to get the current "timestamp". Does this mean that current entiteis trying to get a timestamp during a nested message loop will actually get the outer events timestamp?

Comment 3 by thomasanderson@chromium.org, Aug 3 

Oh, I remember now.  dispatching_event_ used to be a stack of events, but got changed to a single event here:
https://chromium.googlesource.com/chromium/src/+/12871d4be95f2ed2899c35a10332618034ad78d2%5E%21/#F0

The event gets reset when a nested message loop returns, so clients will still get up-to-date timestamps (though there may be a round-trip required).

I don't remember why I did this, though.  All I remember is that it fixed the bug linked on the CL.
►

Sign in to add a comment