Skip to content

Fix poll_for_response race condition

In poll_for_response is it possible that event replies are skipped and a more up to date message reply is returned. This will cause next poll_for_event call to fail aborting the program.

This was proved using some slow ssh tunnel or using some program to slow down server replies (I used a combination of xtrace and strace).

How the race happens:

  • program enters into poll_for_response;
  • poll_for_event is called but the server didn't still send the reply;
  • pending_requests is not NULL because we send a request (see call to append_pending_request in _XSend);
  • xcb_poll_for_reply64 is called from poll_for_response;
  • xcb_poll_for_reply64 will read from server, at this point server reply with an event (say sequence X) and the reply to our last request (say sequence N+1);
  • xcb_poll_for_reply64 returns the reply for the request we asked;
  • last_request_read is set to N+1 sequence in poll_for_response;
  • poll_for_response returns the response to the request;
  • poll_for_event is called (for instance from another poll_for_response);
  • event with sequence N is retrieved;
  • the N sequence is widen, however, as the "new" number computed from last_request_read is less than N the number is widened to N + 2^32 (assuming last_request_read is still contained in 32 bit);
  • poll_for_event enters the nested if statement as req is NULL;
  • we compare the widen N (which now does not fit into 32 bit) with request (which fits into 32 bit) hitting the throw_thread_fail_assert.

To avoid the race condition and to avoid the sequence to go back I check again for new events after getting the response and return this last event if present saving the reply to return it later.

To test the race and the fix it's helpful to add a delay (I used a "usleep(5000)") before calling xcb_poll_for_reply64.

Signed-off-by: Frediano Ziglio fziglio@redhat.com

Edited by Peter Hutterer

Merge request reports