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

Issue when interrupting adb with output captured #2873

Closed
rom1v opened this issue Dec 11, 2021 · 1 comment
Closed

Issue when interrupting adb with output captured #2873

rom1v opened this issue Dec 11, 2021 · 1 comment

Comments

@rom1v
Copy link
Collaborator

rom1v commented Dec 11, 2021

Environment

  • OS: Linux (Debian sid)
  • scrcpy version: 1.21
  • installation method: manual build
  • device model: any
  • Android version: any

Related to #2870, interrupting an adb get-serialno process with Ctrl+c does not close scrcpy immediately in all situations.

To reproduce easily, create an adb wrapper (called slowadb for this example), and make it executable (chmod +x slowadb):

#!/bin/bash
echo slowadb pid=$$ >&2
sleep 10
adb "$@"
echo showadb done >&2

Then use it for scrcpy:

export ADB=./slowadb
scrcpy

Pressing Ctrl+c does not interrupt it.

Analysis

SDL masks many signals, but this is expected: we do not want to receive SIGINT from other threads, and we want to always quit properly (for example, killing while recording could result in a corrupted file). And the solution must work on all platforms anyway.

Instead, to always quit properly, on Ctrl+c, SDL generates a SQL_QUIT event on the main thread, causing sc_server_stop() to be called, which in turn interrupts any blocking call on the server thread.

The problem is that even if the adb process (here, our bash script) is killed, the blocking read from the pipe is not interrupted (it is expected to return -1 immediately instead).

Even killing it from a separate terminal (during the sleep 10) does not help (the pipe read is still blocked), and the sleep is not even interrupted, which is very surprising.

However, if we use a native binary (instead of a shell script calling sleep) to simulate a blocking adb call:

#include <stdio.h>
#include <unistd.h>
int main() {
    fprintf(stderr, "begin\n");
    sleep(10);
    fprintf(stderr, "end\n");
    return 0;
}
gcc a.c
export ADB=./a.out
scrcpy

In that case, it is interrupted immediately (as expected):

scrcpy 1.21 <https://github.com/Genymobile/scrcpy>
begin
DEBUG: Screensaver enabled
^CDEBUG: User requested to quit
DEBUG: Interrupting process
ERROR: Could not get device serial

A pipe read should return immediately when the other end is closed. So if it isn't, then the other end is not closed (probably due to a fork in the child process which keeps it open, and remains open even when the direct child process is killed).

I don't know how to fix it yet. Oh in fact this is also fixed by #2870.

A workaround could be to first wait for process success, then read the output pipe (in other words, reverse the calls here): the process is correctly interrupted on Ctrl+c, contrary to the pipe read. For such small output size, it works, but in the general case this is not correct: a process might be blocked if its stdout is full, causing a deadlock (the caller waits for the process to terminate before consuming its output, but the process is blocked on its output until it is consumed).

rom1v pushed a commit that referenced this issue Dec 11, 2021
Since commit 0426708, the server is run
in a dedicated thread. For SDL, many signals, including SIGINT and
SIGTERM, are masked for new threads. As a result, if the adb server is
not already running, adb commands invoked by scrcpy will start an adb
server that ignores those signals and cannot be terminated at system
shutdown.

Fixes #2873 <#2873>
PR #2870 <#2870>

Signed-off-by: Romain Vimont <rom@rom1v.com>
@rom1v
Copy link
Collaborator Author

rom1v commented Dec 11, 2021

Fixed by #2870.

@rom1v rom1v closed this as completed Dec 11, 2021
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

No branches or pull requests

1 participant