Skip to content

fix(linux): remove hourly --server restart that destabilizes desktop sessions#14935

Open
julianjc84 wants to merge 1 commit into
rustdesk:masterfrom
julianjc84:fix/upstream-remove-hourly-restart
Open

fix(linux): remove hourly --server restart that destabilizes desktop sessions#14935
julianjc84 wants to merge 1 commit into
rustdesk:masterfrom
julianjc84:fix/upstream-remove-hourly-restart

Conversation

@julianjc84

@julianjc84 julianjc84 commented Apr 30, 2026

Copy link
Copy Markdown

Problem

The Linux --service process killed and restarted the --server subprocess every 3600s (or 60s after connection manager closed). This caused:

  • IPC disconnection errors every hour (ipc connection closed: reset by the peer)
  • Tray icon churn (XAppStatusIcon removed and re-registered each cycle)
  • Desktop instability over long uptime on some compositors

The tray icon is owned by --server. Killing it hourly tears down the icon's connection to the tray manager and re-registers a fresh one — visible flicker and, on some compositors, leaked references. With this fix the tray icon lives for the whole session.

Root cause

The restart was a workaround for stale DNS and X display detection. Both are already handled inline:

  • DNS: rendezvous_mediator.rs rebinds the UDP socket after consecutive registration failures (DNS_INTERVAL = 60s)
  • Displays: display_service.rs polls for display changes every 300ms

So the restart is redundant and harmful.

Change

  • Drop the 3600s timer in start_os_service (src/platform/linux.rs)
  • Remove now-unused get_cm(), cm0/last_restart params, Instant import

Test

Verified on Linux: no IPC errors, stable tray.

Summary by CodeRabbit

  • Bug Fixes
    • Simplified Linux service restart handling to avoid periodic hourly restarts and related session-based restart triggers.
    • Server restarts now occur only when display, headless, or seat-change conditions require them.
    • Improved timing import usage for cleaner runtime behavior.

@coderabbitai

coderabbitai Bot commented Apr 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Removes the hourly restart workaround from the Linux service loop. The get_cm() helper, cm0/last_restart state variables, and the elapsed-time/terminal-session restart gating are deleted. should_start_server is simplified to restart only on display/headless/seat-change conditions, and the Duration/Instant import is cleaned up.

Changes

Linux service loop hourly restart removal

Layer / File(s) Summary
should_start_server signature and body
src/platform/linux.rs
Removes cm0 and last_restart parameters, drops the hourly restart block with elapsed-time and terminal-session checks, and removes the *last_restart = Instant::now() update from the return path. Also adjusts the std::time import to drop Instant.
start_os_service call sites and get_cm() deletion
src/platform/linux.rs
Removes cm0/last_restart variable initialization from the service loop, updates both should_start_server(...) call sites to omit those arguments, and deletes the get_cm() private helper.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • fufesou

Poem

🐇 Hop, hop, no more ticking clock,
The hourly restart? Gone from the flock!
get_cm deleted, cm0 erased,
The service loop runs at a simpler pace.
Less state to carry, less code to fear —
The rabbit hops on, the path is clear! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main Linux change: removing the hourly --server restart that caused desktop instability.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@rustdesk

rustdesk commented Jun 3, 2026

Copy link
Copy Markdown
Owner

#15176

The --service process killed and restarted the --server subprocess every
3600 seconds (or 60 seconds after connection manager closed) as a
workaround for stale DNS resolution and X display detection failures.

Both issues are already handled inline without a process restart:
- DNS: rendezvous_mediator.rs rebinds the UDP socket after consecutive
  registration failures (rebind_udp_for, DNS_INTERVAL = 60s)
- Displays: display_service.rs polls for display changes every 300ms
  via check_get_displays_changed_msg()

The hourly restart caused:
- IPC disconnection errors every hour ("ipc connection closed: reset by
  the peer") logged in ui_interface.rs
- Tray icon removed and re-registered each cycle (XAppStatusIcon churn)
- _NET_ACTIVE_WINDOW messages with timestamp 0 on each server startup
- Memory accumulation over multi-day uptime (peaked at 905MB)
- Cinnamon/Muffin instability leading to general protection faults and
  fallback to Metacity after days of continuous operation

Also removes the now-unused get_cm() function, cm0/last_restart
parameters, and Instant import.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: julian <julian@jc84.com>
@julianjc84
julianjc84 force-pushed the fix/upstream-remove-hourly-restart branch from e3410e0 to 887043a Compare June 28, 2026 10:15
@julianjc84
julianjc84 marked this pull request as ready for review June 30, 2026 07:03

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/platform/linux.rs (1)

899-916: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Keep get_cm() until the remaining caller is updatedsrc/ui.rs:170 still calls inline::get_cm(), so removing this helper will break the inline UI path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/platform/linux.rs` around lines 899 - 916, Keep the existing helper used
by the inline UI path until the remaining caller is migrated. The
`inline::get_cm()` reference from `get_cm()` in `src/ui.rs` still depends on
this Linux-side logic, so do not remove or bypass it yet; instead, preserve the
current `is_login_wayland()`-related support and ensure any refactor keeps
`get_cm()` available for that caller.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/platform/linux.rs`:
- Around line 899-916: Keep the existing helper used by the inline UI path until
the remaining caller is migrated. The `inline::get_cm()` reference from
`get_cm()` in `src/ui.rs` still depends on this Linux-side logic, so do not
remove or bypass it yet; instead, preserve the current
`is_login_wayland()`-related support and ensure any refactor keeps `get_cm()`
available for that caller.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 313b8768-60f0-419f-83fc-92affa8ed48b

📥 Commits

Reviewing files that changed from the base of the PR and between 10d5250 and 887043a.

📒 Files selected for processing (1)
  • src/platform/linux.rs

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.

2 participants