Skip to content

Commit 549baca

Browse files
committed
Keep remote-desktop receive threads alive and handshake off the accept thread
Malformed input or a bad WebSocket frame killed the host's receive thread without stop(), leaving a handler that held a client slot, and the viewer's thread died without telling the GUI. The TLS, WS and auth handshake ran on the single accept thread, so one slow peer blocked every other viewer; each connection now gets its own bounded handshake thread. Signaling timeouts and hang-ups are raised as SignalingError.
1 parent acf137a commit 549baca

10 files changed

Lines changed: 218 additions & 55 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,13 @@ it shipped into a `## [x.y.z] - date` section of their own; the tag's
150150

151151
### Fixed
152152

153+
- **Remote desktop keeps serving after bad input.** A malformed INPUT
154+
message or WebSocket frame no longer kills the host's receive thread while
155+
its viewer keeps a client slot, nor the viewer's thread without an error
156+
callback; a slow or silent peer no longer blocks other viewers from
157+
connecting (each connection is handshaken on its own thread); and a
158+
signaling timeout or hang-up is reported as `SignalingError`.
159+
153160
- **Generated code cannot run what an action file smuggles in.** Codegen
154161
emits a parameter as a keyword argument only when its name is a plain
155162
identifier, and the Robot target carries the actions base64-encoded.

‎architecture_explore.md‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ iOS(WebDriverAgent)。核心能力是滑鼠/鍵盤控制、影像辨識、
2020
| 指標 | 數值 |
2121
| --- | ---: |
2222
| Python 模組總數(含周邊子專案) | 1,045 |
23-
| 程式碼總行數 | 143,227 |
23+
| 程式碼總行數 | 143,290 |
2424
| `je_auto_control/utils/` 子套件數 | 310 |
2525
| `AC_*` 動作指令數(`known_commands()` 實測) | 773 |
2626
| 套件門面 `__all__` 公開名稱數 | 1,239 |
@@ -512,14 +512,14 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。
512512

513513
### 5.4.10 遠端桌面與 USB
514514

515-
> 6 個套件、約 18,363 行。
515+
> 6 個套件、約 18,426 行。
516516
517517
| 模組 | 行數 | 職責 |
518518
| --- | ---: | --- |
519519
| `utils/admin/` | 349 | 多主機管理主控台:平行輪詢 N 個 AutoControl REST 端點 |
520520
| `utils/config_sync/` | 246 | 透過訊令伺服器做跨機器設定同步 |
521521
| `utils/device_matrix/` | 138 | 行動裝置矩陣:同一 action list 於多台裝置平行執行 |
522-
| `utils/remote_desktop/` | 12,265 | **遠端桌面子系統**(56 檔/11.7K LOC):TCP/WebSocket/WebRTC 三條傳輸路徑、主機與檢視端、訊令伺服器、TURN/中繼、多檢視者、錄影、信任清單、TOTP、稽核鏈 |
522+
| `utils/remote_desktop/` | 12,328 | **遠端桌面子系統**(56 檔/11.7K LOC):TCP/WebSocket/WebRTC 三條傳輸路徑、主機與檢視端、訊令伺服器、TURN/中繼、多檢視者、錄影、信任清單、TOTP、稽核鏈 |
523523
| `utils/usb/` | 4,422 | 跨平台 USB 列舉/熱插拔/裝置直通(WinUSB、IOKit、libusb 後端 + ACL + WebRTC DataChannel 通道) |
524524
| `utils/usbip/` | 943 | USB/IP 線路協定主機端(協定封包、TCP 伺服器、libusb URB 後端) |
525525

@@ -739,25 +739,25 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。
739739
| `rate_limit.py` | 48 | 工具呼叫的 token bucket 限流。 |
740740
| `__main__.py` | 88 | `je_auto_control_mcp` console script 進入點。 |
741741

742-
#### `utils/remote_desktop/`(12,265 行/56 檔)
742+
#### `utils/remote_desktop/`(12,328 行/56 檔)
743743

744744
三條傳輸路徑並存:**TCP**(JPEG 影格)、**WebSocket**(同協定換傳輸)、**WebRTC**(aiortc 視訊 + DataChannel)。
745745

746746
| 檔案 | 行數 | 職責 |
747747
| --- | ---: | --- |
748748
| `webrtc_host.py` | 716 | WebRTC 主機:串流螢幕視訊並接受檢視端輸入;session 生命週期、DataChannel 接線、檔案收發。 |
749749
| `webrtc_viewer.py` | 672 | WebRTC 檢視端:接收視訊並送出輸入。 |
750-
| `host.py` | 634 | TCP 主機:接受迴圈、TLS 包裝、連線/認證握手、音訊與剪貼簿廣播、檔案推送、單次 token。 |
750+
| `host.py` | 667 | TCP 主機:接受迴圈、TLS 包裝、連線/認證握手、音訊與剪貼簿廣播、檔案推送、單次 token。 |
751751
| `viewer.py` | 634 | TCP 檢視端。 |
752752
| `host_service.py` | 558 | 無頭 WebRTC 主機執行器 + 多平台服務安裝器。 |
753-
| `host_client.py` | 406 | TCP 主機的每連線處理器:一個檢視端一個實例,擁有它的認證交換、sender/audio/receiver 三條執行緒,以及入站訊息的路由表。 |
753+
| `host_client.py` | 417 | TCP 主機的每連線處理器:一個檢視端一個實例,擁有它的認證交換、sender/audio/receiver 三條執行緒,以及入站訊息的路由表。 |
754754
| `registry.py` | 370 | `AC_remote_*` 指令使用的行程級單例。 |
755755
| `webrtc_transport.py` | 369 | 共用 WebRTC 管線:asyncio 橋接執行緒、螢幕視訊軌、設定。 |
756756
| `multi_viewer.py` | 339 | 每個連入檢視端各跑一個 `WebRTCDesktopHost` 的協調器。 |
757757
| `signaling_server.py` | 313 | 獨立的 WebRTC SDP 交換 rendezvous 服務。 |
758758
| `audit_log.py` | 302 | SQLite 雜湊鏈稽核記錄。 |
759759
| `host_capture.py` | 297 | TCP 主機的影格與游標產生:螢幕列舉、監視器索引轉擷取區域、預設 JPEG/游標 provider,以及 `FrameProductionMixin`(游標輪詢、擷取迴圈、上線編碼)。 |
760-
| `ws_protocol.py` | 277 | 最小 RFC 6455 WebSocket 框架與握手。 |
760+
| `ws_protocol.py` | 284 | 最小 RFC 6455 WebSocket 框架與握手。 |
761761
| `file_transfer.py` | 323 | 分塊檔案傳輸。 |
762762
| `relay.py` | 307 | NAT 穿透失敗時的 TCP 中繼。 |
763763
| `fingerprint.py` | 246 | TOFU 主機指紋驗證。 |
@@ -775,10 +775,10 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。
775775
| `webrtc_stats.py` | 167 | 把 aiortc 的 `RTCStats` 報告輪詢成精簡 dict。 |
776776
| `connect_coordinator.py` | 149 | 由使用者輸入的目標決定該用哪條傳輸。 |
777777
| `adaptive_bitrate.py` | 148 | 依統計調整主機擷取 FPS。 |
778-
| `signaling_client.py` | 145 | 純標準庫的訊令用戶端。 |
778+
| `signaling_client.py` | 151 | 純標準庫的訊令用戶端。 |
779779
| `trust_list.py` | 140 | 自動接受的檢視端信任清單。 |
780780
| `webrtc_inspector.py` | 138 | 行程級的 `StatsSnapshot` 滾動視窗。 |
781-
| `input_dispatch.py` | 133 | 在主機端套用輸入訊息。 |
781+
| `input_dispatch.py` | 139 | 在主機端套用輸入訊息。 |
782782
| `session_recorder.py` | 134 | 以 PyAV 把 WebRTC 影格錄成 mp4。 |
783783
| `totp.py` | 140 | RFC 6238 TOTP(零外部相依)。 |
784784
| `file_sync.py` | 141 | 輪詢式資料夾鏡像。 |
@@ -1061,7 +1061,7 @@ socket 預設綁 `127.0.0.1`;資源一律用 `with`。
10611061
| --- | ---: | ---: |
10621062
| `gui/` | 91 | 26,794 |
10631063
| `utils/mcp_server/` | 31 | 17,538 |
1064-
| `utils/remote_desktop/` | 56 | 12,265 |
1064+
| `utils/remote_desktop/` | 56 | 12,328 |
10651065
| `utils/executor/` | 7 | 9,238 |
10661066
| `utils/usb/` | 17 | 4,422 |
10671067
| `je_auto_control/`(頂層 3 檔) | 3 | 2,388 |
@@ -1080,5 +1080,5 @@ socket 預設綁 `127.0.0.1`;資源一律用 `with`。
10801080
| `autocontrol-lsp/` | 8 | 744 |
10811081
| `utils/hotkey/` | 7 | 783 |
10821082
| 其餘模組(約 286 個 `utils/` 子套件 + `android/`/`ios/`/周邊小工具) | 674 | 48,508 |
1083-
| **總計** | **1,039** | **143,162** |
1083+
| **總計** | **1,039** | **143,225** |
10841084

‎docs/updates/2026-09.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,3 +398,13 @@ Index and query commands: [README.md](README.md). New entries go at the end.
398398
- **Tests**: `test_codegen_injection.py` (new, 4; all fail on the previous commit), `test_network_edge_audit.py` (new, 6; it does not import on the previous commit, `atomic_write_bytes` being new). `test_codegen.py`, `test_usbip.py` and `test_r3_rdusb_usbip.py` updated as described.
399399
- **Files**: `je_auto_control/utils/{codegen/codegen,usbip/server,tls_acme/keys,json_store/json_store,remote_desktop/signaling_server,remote_desktop/relay}.py`, `test/unit_test/headless/{test_codegen_injection,test_network_edge_audit,test_codegen,test_usbip,test_r3_rdusb_usbip}.py`, `CHANGELOG.md`, `architecture_explore.md` (figures).
400400
- **Open items**: none.
401+
402+
## U-20260923-30 · 2026-09-23 · Remote-desktop robustness: dead receive threads, blocked accept thread, signaling errors · #incident #remote-desktop
403+
404+
- **What**: The robustness findings of the remote-desktop network audit (the security ones are U-20260923-29).
405+
- **Receive threads**: (1) an INPUT message with a missing field (`KeyError`), an infinite or non-numeric coordinate (`OverflowError` / `ValueError` from `int()`), or an unknown key or button name (the wrappers' `AutoControlException`) escaped the host's catch; the receive thread died without `stop()`, so the handler still counted as an authenticated client, ignored all later input and — with `max_clients=1` — locked out every new viewer. `dispatch_input` turns conversion errors into `InputDispatchError`, the host catches the wrappers' framework errors, and `_recv_loop` calls `stop()` before letting anything else propagate. (2) `WsProtocolError` was a bare `RuntimeError`, so one TEXT or fragmented frame killed the WebSocket host's receive thread the same way and the viewer's silently (`on_error` never ran); it is a `ProtocolError`, which both loops already end a connection on.
406+
- **Accept thread**: the TLS handshake, the WS upgrade and authentication ran on the one accept thread, each read bounded by a 60 s *per-read* timeout, so a peer sending a byte just inside each timeout kept everyone else from connecting (8.6 s and counting in the reproduction). Each connection is handshaken on its own `rd-handshake` thread, at most 32 at once; beyond that new connections are closed.
407+
- **Signaling client**: a read timeout (`TimeoutError`) or a server hanging up (`RemoteDisconnected`) is no `URLError`, so they left `_request` as themselves while the GUI workers catch only `SignalingError`; they are wrapped too.
408+
- **Tests**: `test_rd_robustness_audit.py` (new, 8; all fail on the previous commit). The 360 existing remote-desktop tests pass unchanged, including the two that pin the stop-during-handshake window.
409+
- **Files**: `je_auto_control/utils/remote_desktop/{host,host_client,input_dispatch,ws_protocol,signaling_client}.py`, `test/unit_test/headless/test_rd_robustness_audit.py`, `CHANGELOG.md`, `architecture_explore.md` (figures).
410+
- **Open items**: none.

‎docs/updates/README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ In the same commit: delete the item from `Progress.md`, add a `#done` entry here
5858

5959
| ID | Date | Title | Tags | Batch |
6060
|---|---|---|---|---|
61+
| U-20260923-30 | 2026-09-23 | Remote-desktop robustness: dead receive threads, blocked accept thread, signaling errors | #incident #remote-desktop | [2026-09](2026-09.md) |
6162
| U-20260923-29 | 2026-09-23 | Codegen injection, USB/IP device scope, TLS key writes, signaling and relay limits | #incident #security | [2026-09](2026-09.md) |
6263
| U-20260923-28 | 2026-09-23 | Shared JSON stores lock and re-read across processes | #done #stores | [2026-09](2026-09.md) |
6364
| U-20260923-27 | 2026-09-23 | CLI / hotkey / recording audit: exit codes, lost recordings, wrong virtual keys | #incident #cli #hotkey | [2026-09](2026-09.md) |
@@ -134,7 +135,7 @@ In the same commit: delete the item from `Progress.md`, add a `#done` entry here
134135

135136
| File | Period | Entries |
136137
|---|---|---:|
137-
| [2026-09.md](2026-09.md) | 2026-09 | 45 |
138+
| [2026-09.md](2026-09.md) | 2026-09 | 46 |
138139
| [2026-08-f.md](2026-08-f.md) | 2026-08 | 1 |
139140
| [2026-08-e.md](2026-08-e.md) | 2026-08 | 2 |
140141
| [2026-08-d.md](2026-08-d.md) | 2026-08 | 2 |

‎je_auto_control/utils/remote_desktop/host.py‎

Lines changed: 68 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,17 @@ def _validate_host_args(token: str, fps: float, quality: int) -> None:
6969
raise ValueError("quality must be in [1, 95]")
7070

7171

72+
#: Connections being handshaken at once; beyond this new ones are closed.
73+
_MAX_HANDSHAKES = 32
74+
75+
76+
def _close_quietly(sock: socket.socket) -> None:
77+
try:
78+
sock.close()
79+
except OSError:
80+
pass
81+
82+
7283
class RemoteDesktopHost(FrameProductionMixin):
7384
"""Stream the screen to authenticated viewers and apply their input.
7485
@@ -176,6 +187,7 @@ def __init__(
176187
self._lifecycle_lock = threading.RLock()
177188
self._clients: List[_ClientHandler] = []
178189
self._clients_lock = threading.Lock()
190+
self._handshake_slots = threading.BoundedSemaphore(_MAX_HANDSHAKES)
179191
self._frame_cond = threading.Condition()
180192
self._latest_frame: Optional[bytes] = None
181193
self._latest_seq = 0
@@ -546,45 +558,66 @@ def _accept_loop(self, stop: threading.Event) -> None:
546558
except OSError:
547559
return
548560
if not self._ip_allowed(address):
549-
try:
550-
client_sock.close()
551-
except OSError:
552-
pass
561+
_close_quietly(client_sock)
553562
continue
563+
# The TLS handshake, the WS upgrade and authentication each wait
564+
# up to _AUTH_TIMEOUT_S *per read*, and ran here, on the one
565+
# accept thread: a peer sending a byte just inside each timeout
566+
# kept every other viewer from connecting. Each connection now
567+
# gets its own thread, and a bounded number of them at once.
568+
if not self._handshake_slots.acquire(blocking=False):
569+
autocontrol_logger.info(
570+
"remote_desktop dropping %s: too many handshakes in progress",
571+
address,
572+
)
573+
_close_quietly(client_sock)
574+
continue
575+
threading.Thread(
576+
target=self._handshake, args=(client_sock, address, stop),
577+
name="rd-handshake", daemon=True,
578+
).start()
579+
580+
def _handshake(self, client_sock: socket.socket, address,
581+
stop: threading.Event) -> None:
582+
"""Open the channel, register the client, then authenticate it."""
583+
try:
554584
channel = self._open_channel(client_sock, address)
555585
if channel is None:
556-
continue
586+
return
557587
handler = _ClientHandler(self, channel, address)
558-
# Prune handlers whose viewer already disconnected *before* the
559-
# capacity check below. Otherwise a client table filled with
560-
# max_clients dead handlers rejects every new connection forever:
561-
# the reject branch continues without ever reaping (the reap used
562-
# to run only on the accept-success path).
563-
self._reap_dead_clients()
564-
with self._clients_lock:
565-
# Re-check under the lock. _open_channel above performs the
566-
# auth/TLS handshake, which can take up to _AUTH_TIMEOUT_S; a
567-
# stop() during that window sets _shutdown and then snapshots
568-
# and clears _clients under this same lock. Without this check
569-
# the handler is registered *after* that snapshot, so nothing
570-
# ever stops it — leaving a viewer dispatching input to a host
571-
# the operator has already stopped.
572-
if stop.is_set():
573-
autocontrol_logger.info(
574-
"remote_desktop dropping %s: host stopped during "
575-
"handshake", address,
576-
)
577-
handler._close()
578-
return
579-
if len(self._clients) >= self._max_clients:
580-
autocontrol_logger.info(
581-
"remote_desktop dropping %s: max_clients reached",
582-
address,
583-
)
584-
handler._close()
585-
continue
586-
self._clients.append(handler)
587-
handler.start()
588+
if self._register_client(handler, address, stop):
589+
handler.start()
590+
finally:
591+
self._handshake_slots.release()
592+
593+
def _register_client(self, handler: "_ClientHandler", address,
594+
stop: threading.Event) -> bool:
595+
"""Add ``handler`` to the client table unless stopped or full."""
596+
# Prune handlers whose viewer already disconnected *before* the
597+
# capacity check below. Otherwise a client table filled with
598+
# max_clients dead handlers rejects every new connection forever.
599+
self._reap_dead_clients()
600+
with self._clients_lock:
601+
# Re-check under the lock: a stop() during the handshake sets
602+
# _shutdown and then snapshots and clears _clients under this same
603+
# lock, so a handler registered after that would never be stopped
604+
# -- a viewer dispatching input to a host the operator stopped.
605+
if stop.is_set():
606+
autocontrol_logger.info(
607+
"remote_desktop dropping %s: host stopped during "
608+
"handshake", address,
609+
)
610+
handler._close()
611+
return False
612+
if len(self._clients) >= self._max_clients:
613+
autocontrol_logger.info(
614+
"remote_desktop dropping %s: max_clients reached",
615+
address,
616+
)
617+
handler._close()
618+
return False
619+
self._clients.append(handler)
620+
return True
588621

589622
def _build_channel(self, sock: socket.socket,
590623
address) -> MessageChannel:

‎je_auto_control/utils/remote_desktop/host_client.py‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import threading
1313
from typing import TYPE_CHECKING, Deque, Optional
1414

15+
from je_auto_control.utils.exception.exceptions import AutoControlException
1516
from je_auto_control.utils.logging.logging_instance import autocontrol_logger
1617
from je_auto_control.utils.remote_desktop.auth import make_nonce
1718
from je_auto_control.utils.remote_desktop.clipboard_sync import (
@@ -81,10 +82,10 @@ def start(self) -> None:
8182
return
8283
self.authenticated = True
8384
# The initial cursor + frame are seeded from _send_loop (the
84-
# per-client sender thread), not here. start() runs on the shared
85-
# accept thread with the socket timeout already cleared, so sending a
86-
# full-screen JPEG to a viewer that authenticates then stops reading
87-
# would block every new accept until its send buffer drains.
85+
# per-client sender thread), not here: start() runs on this
86+
# connection's handshake thread with the socket timeout already
87+
# cleared, and a viewer that authenticates then stops reading would
88+
# pin that thread -- and its handshake slot -- until its buffer drains.
8889
self._sender_thread = threading.Thread(
8990
target=self._send_loop, name="rd-sender", daemon=True,
9091
)
@@ -267,7 +268,14 @@ def _recv_loop(self) -> None:
267268
)
268269
self.stop()
269270
return
270-
self._route_incoming(msg_type, payload)
271+
try:
272+
self._route_incoming(msg_type, payload)
273+
except BaseException:
274+
# Dying here without stop() left a handler that still
275+
# counted as an authenticated client: it ignored all later
276+
# input and held a slot that locked out new viewers.
277+
self.stop()
278+
raise
271279

272280
def _route_incoming(self, msg_type: MessageType, payload: bytes) -> None:
273281
"""Dispatch one received message to the matching handler."""
@@ -396,7 +404,10 @@ def _handle_input_payload(self, payload: bytes) -> None:
396404
"remote_desktop rejected INPUT from %s: %r",
397405
self._address, error,
398406
)
399-
except (OSError, RuntimeError, ValueError, TypeError) as error:
407+
# AutoControlException: the wrappers raise it for an unknown key or
408+
# button name, which is the peer's input, not the host failing.
409+
except (OSError, RuntimeError, ValueError, TypeError,
410+
AutoControlException) as error:
400411
autocontrol_logger.warning(
401412
"remote_desktop input apply failed for %s: %r",
402413
self._address, error,

‎je_auto_control/utils/remote_desktop/input_dispatch.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,13 @@ def dispatch_input(message: Mapping[str, Any]) -> Any:
5757
if action == "ping":
5858
return None
5959
wrappers = _import_wrappers()
60-
return _APPLIERS[action](message, wrappers)
60+
try:
61+
return _APPLIERS[action](message, wrappers)
62+
except (KeyError, TypeError, ValueError, OverflowError) as error:
63+
# A missing field (KeyError), a non-numeric or infinite coordinate
64+
# (ValueError / OverflowError from int()) is the peer's malformed
65+
# message, and the receive loop only expects this type for that.
66+
raise InputDispatchError(f"malformed {action!r} message: {error!r}") from error
6167

6268

6369
def _apply_mouse_move(message: Mapping[str, Any], wrappers: Dict[str, Any]) -> Any:

0 commit comments

Comments
 (0)