fix(tty,vt,console): prevent deadlocks and hangs in terminal I/O#3420
Open
michaellukashov wants to merge 1 commit into
Open
fix(tty,vt,console): prevent deadlocks and hangs in terminal I/O#3420michaellukashov wants to merge 1 commit into
michaellukashov wants to merge 1 commit into
Conversation
d514eb7 to
9dcf088
Compare
Contributor
|
Interesting. Great work! I encountered hangs when closing far2l, but not in other situations. |
dc427fd to
179dbce
Compare
Orphan detection uses prctl(PR_SET_PDEATHSIG, SIGUSR1) instead of getppid() polling, eliminating the PID reuse race and removing a syscall every 10ms from the suspend loop. Guard tcdrain, clipboard RPC, and Far2lInteract entry with _deadio to prevent indefinite blocking on dead file descriptors during shutdown. Far2lInteract uses a 10s TimedWait with _exiting/_deadio early-out to avoid blocking when the far2l peer disconnects. Fix SIGTSTP/SIGCONT handler swap in WinPortMainTTY cleanup. Two-phase lock in OnConsoleLog: release _inout_control_mutex before the blocking InterThreadCall to prevent main-thread starvation on mouse scroll during TUI app execution. Dispatch inter-thread calls and check pending ctrl events when NOOP_EVENT is received, preventing infinite spin when a delegate dispatch is pending. Drain kickass cancel byte inside LocalSocket::WaitForClient instead of in caller catch blocks to prevent rethrow loops for callers that retry after LocalSocketCancelled.
179dbce to
502a276
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes for six deadlock and indefinite-hang scenarios in the TTY backend, VT
shell, and console input layer. Each fix addresses a distinct root cause where
a thread could block indefinitely, spin-loop at 100% CPU, or hold a mutex
across a blocking modal call.
Changes
1. Console input: dispatch inter-thread calls on
NOOP_EVENTFile:
far2l/src/console/keyboard.cppProblem:
GetInputRecordInnerpeekedNOOP_EVENT(written byInterThreadCallto wake the input queue) but did not recognize it. The peekreturned true, the loop broke out of the inner block,
WaitConsoleInputreturned immediately (NOOP still pending), and the outer loop peeked the same
NOOP again — a tight spin loop burning 100% CPU.
DispatchInterThreadCalls()was never called, starving all inter-thread delegates (including the VT input
reader during mouse scroll).
Fix: Added a
NOOP_EVENThandler inside the peek block: consume the NOOPwith
ReadInput, dispatch pending inter-thread calls, check for pending Ctrlevents, and continue the loop to process the real event now in
rec.2. TTY WriterThread: skip
tcdrain()during shutdownFile:
WinPort/src/Backend/TTY/TTYBackend.cppProblem:
tcdrain(_stdout)blocks until all buffered output istransmitted. When the terminal is disconnected (
_deadio) or the process isexiting (
_exiting), the kernel tty buffer can never drain, causingpthread_joinon the writer thread to hang forever during backend teardown.Fix: Guard
tcdrainwithif (!_exiting && !_deadio).3. TTY Far2lInteract RPC: timeout and shutdown awareness
File:
WinPort/src/Backend/TTY/TTYBackend.cppProblem:
Far2lInteractcalledpfi->evnt.Wait()— an indefinite block.If the far2l peer disconnected mid-RPC, the ReaderThread hung forever.
Additionally, the method did not check
_deadioat entry.Fix: Replaced with a
TimedWaitloop (10 s total timeout, 500 ms pollgranularity). Checks
_exiting || _deadioon entry and on each poll iteration,so terminal disconnect is detected within 500 ms. The 500 ms interval balances
shutdown responsiveness against loop overhead.
4. TTY signal handlers: correct
SIGTSTP/SIGCONTrestoration swapFile:
WinPort/src/Backend/TTY/TTYBackend.cppProblem: In the
WinPortMainTTYcleanup path,SIGCONTwas restored tothe old
SIGTSTPhandler and vice versa. If the original handlers were bothSIG_DFL, this was functionally harmless by coincidence (SIG_DFLdispatchesby signal number). If the parent process had installed non-default handlers,
the swap would cause incorrect terminal state manipulation on exit.
Fix: Swapped the assignments so each signal is restored to its correct
original handler.
5. TTY ReaderThread: detect ForkTTYChild wrapper exit
Files:
WinPort/src/Backend/TTY/TTYBackend.cpp,TTYBackend.h,TTYRevive.cpp,utils/src/LocalSocket.cppProblem: When the
ForkTTYChildwrapper exited (terminal closed / SIGHUP),the orphaned child hung indefinitely in the revival loop —
TTYReviveMewaited for a peer that would never connect because the session manager was gone.
Fix:
_parent_pid(std::atomic<pid_t>, captured at construction) and_is_forktty_child(notify_pipe != -1).getppid() != _parent_pid. Onparent death, set
_exiting, injectCTRL_CLOSE_EVENT, and break out.TTYReviveMecatch block intoLocalSocketServer::WaitForClientat the throw site — all callers benefitfrom automatic drain before
LocalSocketCancelledis thrown, preventingimmediate rethrow on the next
selectiteration.6. VT shell: fix
OnConsoleLogdeadlockFile:
far2l/src/vt/vtshell.cppProblem:
OnConsoleLog(called from the input thread) usedtry_to_lockon
_inout_control_mutex— silently dropping log requests under contention.When it did acquire the lock, it held it across a blocking
InterThreadCall(modal console log dialog). The main thread, which services
InterThreadCall,may need to call
StartIOReaders/StopIOReaders— which also acquire_inout_control_mutex. Classic lock-order deadlock.Fix: Three-phase lock discipline:
InterThreadCallwith no locks heldThis eliminates silent drops (no more
try_to_lock) and the deadlock (mutexreleased before the blocking call). VTA being active during the
InterThreadCallis safe — the output reader is stopped so no ANSI sequencesare parsed, and VTA only gates parsing state, not terminal output.
Testing
ForkTTYChildmode by closingthe terminal window while far2l was running.
tcdraindisabled during teardown.NOOP_EVENTdispatch resolves the input-reader spin loop observedduring rapid mouse scroll in TTY mode.