feat: Add a pure-Rust connection to x11rb-async

[notgull]

Complements #790 by adding a connection type to x11rb-async that connects to the X11 server using pure Rust. I use async-io for handling I/O and async-lock for locking shared types.

By the way, do we also want to have an async XCB-based connection as well? It's possible, see bread-graphics/whitebreadx#1, but it would need blocking I/O anyways so the official direction might need to be "just throw an XCBConnection into a BlockingConnection.

[codecov]

Codecov Report

Patch coverage has no change and project coverage change: -0.19 ⚠️

Comparison is base (15153e5) 12.46% compared to head (4314d8f) 12.28%.

❗ Current head 4314d8f differs from pull request most recent head f402e31. Consider uploading reports for the commit f402e31 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #797      +/-   ##
==========================================
- Coverage   12.46%   12.28%   -0.19%     
==========================================
  Files         180      183       +3     
  Lines      133169   135277    +2108     
==========================================
+ Hits        16603    16615      +12     
- Misses     116566   118662    +2096     

Flag	Coverage Δ
tests	12.28% <0.00%> (-0.19%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
x11rb-async/examples/xclock_utc.rs	0.00% <0.00%> (ø)
x11rb-async/src/lib.rs	100.00% <ø> (ø)
x11rb-async/src/rust_connection/mod.rs	0.00% <0.00%> (ø)
x11rb-async/src/rust_connection/nb_connect.rs	0.00% <0.00%> (ø)
x11rb-async/src/rust_connection/stream.rs	0.00% <0.00%> (ø)
x11rb/src/rust_connection/packet_reader.rs	75.20% <ø> (-0.20%)	⬇️

... and 34 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[psychon]

error: package raw-window-handle v0.5.1 cannot be built because it requires rustc 1.64 or newer, while the currently active rustc version is 1.56.0

Sigh... Can we come back when the Rust ecosystem becomes mature and stops breaking stuff all the time? As far as I can tell, the PR that introduced that picked that version just because they want to eventually use features from that version. Not because they already require this.

Shall we pin the dependency to 0.5.0? 🙈

[psychon]

Google suggests that the appveyor failure is transient. However, a retry of the build did not make it go away. Yay. 🙈

cargo check --verbose --package x11rb --all-targets --features all-extensions,cursor,image
    Updating crates.io index
 Downloading crates ...
warning: spurious network error (2 tries remaining): [35] SSL connect error (schannel: next InitializeSecurityContext failed: SEC_E_ILLEGAL_MESSAGE (0x80090326) - This error usually occurs when a fatal SSL/TLS alert is received (e.g. handshake failed). More detail may be available in the Windows System event log.)
[SNIP - it repeats a bit]
error: failed to download from `https://crates.io/api/v1/crates/gethostname/0.3.0/download`
Caused by:
  [35] SSL connect error (schannel: next InitializeSecurityContext failed: SEC_E_ILLEGAL_MESSAGE (0x80090326) - This error usually occurs when a fatal SSL/TLS alert is received (e.g. handshake failed). More detail may be available in the Windows System event log.)
Command exited with code 101

[psychon]

IIRC the biggest open question here is to do the reading. Right now this implements "automatic reading" in x11rb-style, but at the same time also offers the possibility to do that in an extra task. Right now I do not see the value in providing both and the "automatic reading"-code seems to have some races.

Besides that, I think the other comments are minor and/or clear.

[psychon]

Why not lock()? Whatever this is, it might need a comment. And even then, it seems incorrect: The lock guard is held across an await point in the case of Unknown, so if two large requests are sent at once, one of them could fail incorrectly with a "too large request" error.

[notgull]

The mutex being locked is taken as a signal that another part of the program is using the current request to find out the maximum bytes. I've added a comment clarifying this.

[psychon]

Yeah, but couldn't it be another task that holds this mutex? I don't think using the mutex like this actually works.

So... how does x11rb do it? RustConnection::maximum_request_bytes() unconditionally locks the mutex. This would not work if the current thread already holds the mutex.

Also, the code in compute_length_field only calls maximum_request_bytes() when the request length does not fit into an u16. That feels a bit like a bug (couldn't an X11 server have a maximum request length smaller than 2^18?), but that should mean that all requests from the BigRequests extension do not actually hit this code here...

[notgull]

Personally, I'd prefer to just have a future that's spawned at connect()-time and is polled for ever.

[psychon]

Thanks! Let's see what happens.

I'll merge this so that we do not end in endless discussions. I'll write down the remaining points for later:

• I am not yet convinced the try_lock() for the maximum request length actually works correctly. A "too large request" error is possible just because another task currently holds the mutex.
• Implementing Default for x11rb-protocol::connection::Connection.