[remoting] Wait for PipeWire socket in systemd_user_env_setter [chromium/src : main]
Yuwei Huang (Gerrit)
Yuwei Huang added 3 comments![]( "Open in Gerrit")
socket_name = pipewire_remote_it->second;If `PIPEWIRE_REMOTE` is present in the environment but set to an empty string, `socket_name` will be empty. Later, `runtime_dir.Append("")` will simply return `runtime_dir`. Because `runtime_dir` is an existing directory, `base::PathExists(pw_socket)` will evaluate to `true`, and the check will incorrectly succeed without waiting for the actual socket.
Consider ignoring empty values for `PIPEWIRE_REMOTE` so it falls back to `"pipewire-0"`:
```cpp
if (pipewire_remote_it != env_map.end() && !pipewire_remote_it->second.empty()) {
socket_name = pipewire_remote_it->second;
}
```
LOG(WARNING) << "Cannot find the XDG_RUNTIME_DIR environment variable.";This warning message is slightly inaccurate since it could also be that `PIPEWIRE_RUNTIME_DIR` was missing or empty. Consider updating the message to reflect both variables, or something more generic like `"Cannot determine runtime directory for PipeWire socket."`.
"Timeout waiting for DISPLAY or "Since the loop now also waits for PipeWire socket readiness, this timeout message might be slightly misleading if it actually timed out waiting for PipeWire. Consider updating the message to reflect that we might also be waiting for the PipeWire socket.
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yuwei Huang (Gerrit)
Yuwei Huang added 5 comments![]( "Open in Gerrit")
If `PIPEWIRE_REMOTE` is present in the environment but set to an empty string, `socket_name` will be empty. Later, `runtime_dir.Append("")` will simply return `runtime_dir`. Because `runtime_dir` is an existing directory, `base::PathExists(pw_socket)` will evaluate to `true`, and the check will incorrectly succeed without waiting for the actual socket.
Consider ignoring empty values for `PIPEWIRE_REMOTE` so it falls back to `"pipewire-0"`:
```cpp
if (pipewire_remote_it != env_map.end() && !pipewire_remote_it->second.empty()) {
socket_name = pipewire_remote_it->second;
}
```
Done
if (pipewire_runtime_dir_it != env_map.end()) {Consider checking `!pipewire_runtime_dir_it->second.empty()` here, similar to how it's done for `PIPEWIRE_REMOTE` on line 83. If this variable happens to be present but empty in the systemd environment, it will currently skip falling back to `XDG_RUNTIME_DIR` and cause the `runtime_dir.empty()` check below to fail.
if (xdg_runtime_dir_it != env_map.end()) {Similarly, it's safer to check `!xdg_runtime_dir_it->second.empty()` here to ensure we don't try to use an empty path.
LOG(WARNING) << "Cannot find the XDG_RUNTIME_DIR environment variable.";This warning message is slightly inaccurate since it could also be that `PIPEWIRE_RUNTIME_DIR` was missing or empty. Consider updating the message to reflect both variables, or something more generic like `"Cannot determine runtime directory for PipeWire socket."`.
Done
Since the loop now also waits for PipeWire socket readiness, this timeout message might be slightly misleading if it actually timed out waiting for PipeWire. Consider updating the message to reflect that we might also be waiting for the PipeWire socket.
Done
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yuwei Huang (Gerrit)
Yuwei Huang added 2 comments![]( "Open in Gerrit")
Consider checking `!pipewire_runtime_dir_it->second.empty()` here, similar to how it's done for `PIPEWIRE_REMOTE` on line 83. If this variable happens to be present but empty in the systemd environment, it will currently skip falling back to `XDG_RUNTIME_DIR` and cause the `runtime_dir.empty()` check below to fail.
Done
Similarly, it's safer to check `!xdg_runtime_dir_it->second.empty()` here to ensure we don't try to use an empty path.
Done
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yuwei Huang (Gerrit)
Yuwei Huang voted and added 1 comment![]( "Open in Gerrit")
Votes added by Yuwei Huang
| Auto-Submit | +1 |
| Commit-Queue | +1 |
1 comment
Related details
- Joe Downing
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joe Downing (Gerrit)
Joe Downing voted![]( "Open in Gerrit")
- Yuwei Huang
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
[remoting] Wait for PipeWire socket in systemd_user_env_setter
This is a more or less speculative fix for the bug. I'm not 100% sure
that the PipeWire connection issue is caused by this race condition, but
Gemini's explanation (b/514622552#comment4) seems quite plausible.
In Wayland sessions, the desktop environment might report that the
session is ready before PipeWire has fully initialized its socket. This
race condition can lead to desktop capture failures or black screens
during GDM-to-User session transitions.
This CL introduces a PipeWire socket readiness check during early host
startup. We dynamically resolve the PipeWire socket path (respecting
PIPEWIRE_REMOTE, PIPEWIRE_RUNTIME_DIR, and XDG_RUNTIME_DIR) and poll
until the socket exists before declaring the systemd user environment
ready.
Bug: 514622552
TAG=agy CONV=0ff425f2-6555-46b9-9039-a1510beb3aff
- M remoting/host/linux/systemd_user_env_setter.cc
Code-Review: +1 by Joe Downing
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |