Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

XWindowsEventQueueBuffer: Fix delays when waiting for new events #679

Merged
1 commit merged into from Nov 10, 2021

Conversation

ghost
Copy link

@ghost ghost commented Nov 4, 2021

This PR has been migrated from old Barrier Github repository debauchee/barrier#679

PR created on: 2020-05-20 by @p12tic
PR last updated on: 2020-07-03

Fixes #617, #298, #58, symless/synergy-core#6487.

This PR fixes intermittent waits on Linux client. These were caused by the wait loop in XWindowsEventQueueBuffer where we wait for new events. The problem was that we use QLength() to check if there are new events coming from the X11 server. This is insufficient, because it checks only for messages that the XLib already knows about.

Consider the situation when we have no pending events at all and we enter the wait loop. An event arrives in the socket, we are woken up and go on to check QLength(). It will return 0, because the socket itself was not read by XLib. If we call XPending() instead, the socket would be read as we expect, the event would become visible to XLib, XPending() would return nonzero value and we would exit the event loop.

Tested for a couple of days, did not see problems on my local workstation. The fix need further testing, maybe there are other related issues that I have not noticed.


Commented on: 2020-05-20 by @p12tic

The upstream PR symless/synergy-core#6685 is incorrect because the input timeout is in seconds and we're operating on milliseconds within the function, so a 1000 multiplier is expected. What the upstream PR did was effectively disabled the waiting for events which just burns the CPU. Though it's not full busy-wait, so the impact may not be large.


Commented on: 2020-05-20 by @p12tic

cc @Jnewbon. I think you should consider something similar to the approach in this PR, instead of what's in symless/synergy-core#6685.


Commented on: 2020-05-20 by @shymega

Makes sense. I'd like to give it a bit more time to be tested before merging.

I'm preparing for a v2.3.3 release, but I'm keen to get this patch in the release.


Commented on: 2020-05-22 by @axtux

Hello,

Thanks for the fix. As I understand, it is a fix for the client. I tried your fix client side and it does not solve my problem debauchee/barrier#617 (comment)


Commented on: 2020-05-22 by @p12tic

@axtux Interesting. Does the Barrier "unblock" itself if you move a mouse on the client or do any other input event that does not go through Barrier?

I had similar symptoms caused by poor internet connection, so maybe the root cause is different.


Commented on: 2020-05-22 by @Jnewbon

@p12tic Thanks for the ping, you make total sense with the fix, ill have a look at pulling this in to a test branch as i can reliably reproduce the bug.


Commented on: 2020-05-22 by @axtux

@p12tic Last time I tried, generating an input event did not unblock anything. Though at that time, the lag was shorter and it was quite difficult to test. I should try again.

Right now, I'm trying the symless/synergy-core#6685 fix to see if that solves my issue.


Commented on: 2020-05-22 by @axtux

Also, I can confirm the fix does not solve debauchee/barrier#207 (comment)


Commented on: 2020-05-22 by @p12tic

@axtux: Alright, so it seems that you could have an unreliable network connection between the client and the server. What happens is that corrupted packets will cause the TCP connection between the client and the server to break and then it takes time to re-establish the connection again, which looks like a delay. The issue that I was fixing was that the client sometimes blocks waiting for X11 events even when there are some. If you generate some events by moving mouse on the client, then the client will unblock. This is how we can check whether the issue is caused by network connection problems or by waiting for already arrived events.

I removed #207 from the list of issues I think this bug fixes, because after rereading it also seems to be caused by connection problems. Dropping the connection when a shift or another modifier key is pressed will cause Barrier to lose this information and then it gets stuck.


Commented on: 2020-05-22 by @axtux

@p12tic As stated in this comment debauchee/barrier#617 (comment), both computers are connected together via Ethernet cable, ping running during freeze is never higher than 1ms so this excludes eventual network issue

Also, I confirm this fix symless/synergy-core#6685 fixes the freeze issue on my side.

As per debauchee/barrier#207 (comment), I'll update my comment to state that this happens every time I press the shift key. What are the chances that a network issue happens every time I press this key?


Commented on: 2020-05-22 by @p12tic

@axtux:

Could you modify getPendingCountLocked to this and test again?

int XWindowsEventQueueBuffer::getPendingCountLocked()
{
    Lock lock(&m_mutex);
    // work around a bug in libx11 which causes the first XPending not to read events sometimes
    // https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/1
    XPending(m_display); 
    return XPending(m_display);
}

This could be the missing piece.


Commented on: 2020-05-25 by @linaori

In regards of the keys getting stuck as I opened an issue about this as well... After updating to Ubuntu 20.04 my ethernet started working, so I'm wired now. This is a direct connection between both my PC and laptop via the same switch.

While I've had a lot less disconnects (oddly enough still happening a few times a day), the issue of keys/mouse cursor getting stuck persists. I can't say it's not the network, just hoping that this info will help.


Commented on: 2020-05-25 by @axtux

@p12tic I tried again this PR and the original code fixes the issue 馃槙

Now I'm wondering if I did anything wrong in my first test. I'll do more testing during the week.


Commented on: 2020-05-29 by @p12tic

@shymega Could this PR be merged? It fixes the problem for me and at least one other user I'm in contact with, as well as for @axtux. The problem and the bugfix are well understood and there's little chance for a regression.


Commented on: 2020-05-30 by @shymega

Merging now. Thanks!


Commented on: 2020-07-03 by @richwalker

Apologies, I've commented and attached debug logs here:
debauchee/barrier@b373d8e#commitcomment-40347586


Closed on: 2020-05-30

QLength() may return 0 even if there are events pending because they
need to be read from the display socket in order to become visible. We
must use XPending() which will poll the socket if QLength() == 0.
This pull request was closed.
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.

Input freezes or gets stuck, losing control for a few seconds
1 participant