Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "sshdriver: Prevent timeout from deadlock" #1288

Commits on Oct 20, 2023

  1. Revert "sshdriver: Prevent timeout from deadlock"

    This reverts commit b8f059f.
    
    OpenSSH < 8.5 called with -f waits for the stderr pipe to be closed
    before forking. [1] fixes this behavior.
    The labgrid commit to be reverted calls communicate(), relying on a
    timely fork. Affected OpenSSH versions now run into a TimeoutExpired
    exception on communicate() leading to an ExecutionError being raised in
    labgrid.
    
    A wait() with timeout was used since the initial implementation [2].
    We wanted to make sure that we don't depend on the state of the pipes, so
    use of wait() was intentional, as it directly covers the interesting cases:
    
    - immediate abort (due to a config error or similar)
    - normal startup (parent process exits after fork)
    - hang (via wait() timeout)
    
    Reverting the problematic commit avoids the complexity of having to
    maintain two different ways to start SSH as suggested in [3].
    
    If timeouts still occur after the revert, we should implement this
    suggestion [4].
    
    Discussion that lead to this patch can be found in the PR introducing
    the problematic commit [5] as well as in [3].
    
    The other commit [6] in the PR [5] has a similar approach for
    `labgrid.util.SSHConnection._start_own_master()`. It shouldn't
    be problematic though: The ssh process is not expected to fork as it is
    not called with -f.
    
    [1] OpenSSH: 396d32f3a ("upstream: There are lots of place where we want to redirect stdin,")
    [2] e4862fa ("SSHDriver: ControlMaster & Driver")
    [3] labgrid-project#1278
    [4] labgrid-project#1265 (comment)
    [5] labgrid-project#1265
    [6] f9ca024 ("ssh: Prevent timeout from deadlock")
    
    Signed-off-by: Bastian Krause <[email protected]>
    Bastian-Krause committed Oct 20, 2023
    Configuration menu
    Copy the full SHA
    d425be4 View commit details
    Browse the repository at this point in the history