Skip to content

Fix: Check for remaining events in the internal X11/xcb buffers#86

Merged
micahrj merged 2 commits intoRustAudio:masterfrom
WeirdConstructor:fix_resize_problems
Mar 4, 2021
Merged

Fix: Check for remaining events in the internal X11/xcb buffers#86
micahrj merged 2 commits intoRustAudio:masterfrom
WeirdConstructor:fix_resize_problems

Conversation

@WeirdConstructor
Copy link
Copy Markdown
Contributor

I went to #xcb on Freenode and psychon was so kind to point me to:
https://docs.rs/x11rb/0.8.0/x11rb/event_loop_integration/index.html

It seems that while events are being handled new events can be queued up
in the internal buffers. And so the next poll() will wait even though there are
still events to be handled.

This is a race condition, so that might be why this does only occur on some systems.

@psychon
Copy link
Copy Markdown

psychon commented Jan 9, 2021

I also hacked together an example that reproduces the problem and hopefully makes it clearer what is going on. Since I don't know where to leave it, I'll leave it here: https://gist.github.com/psychon/6fc6a6ed06ddfc5eea5f719aac14b7bf

The example creates a window, sends an event to the window, then does get_input_focus().get_reply(). This last part ensures that the event was already received by xcb and the FD is no longer readable.
Then, I call poll() and print the information that the FD really is not readable, but poll_for_event() still returns an event afterwards.

@WeirdConstructor
Copy link
Copy Markdown
Contributor Author

I want to note: This is the cause of the resizing issues I've had, so this is not only theoretical.

@micahrj
Copy link
Copy Markdown
Member

micahrj commented Mar 4, 2021

This fix looks correct to me. To make it airtight, the logic in drain_xcb_events also needs to be restructured so that poll_for_events returning None is the last thing that happens before it returns.

@micahrj micahrj merged commit 50c4175 into RustAudio:master Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants