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

Improve cleanup reliability #5613

Merged
merged 2 commits into from
Dec 8, 2024
Merged

Improve cleanup reliability #5613

merged 2 commits into from
Dec 8, 2024

Conversation

rom1v
Copy link
Collaborator

@rom1v rom1v commented Dec 5, 2024

Currently, if scrcpy terminates before the cleanup process is started (race condition), e.g. due to an early error, the initial state is not restored. This can be easily reproduced by forcing scrcpy to fail:

scrcpy --show-touches --video-encoder=fail

In addition, starting the cleanup process with setsid or nohup reduces the likelihood of it being killed along with the scrcpy server process on some devices (but not all).

Refs #5601.

Here is a binary to replace in scrcpy 3.0.2:

  • scrcpy-server SHA-256: e925077940dbe82ab8375aca06bf471d88a7c775096b6747eac810f074481b9

rom1v added 2 commits December 8, 2024 10:58
Some options, such as --show-touches or --stay-awake, modify Android
settings and must be restored upon exit.

If scrcpy terminates (e.g. due to an early error) in the middle of the
clean up configuration, the device may be left in an inconsistent state
(some settings might be changed but not restored).

This issue can be reproduced with high probability by forcing scrcpy to
fail:

    scrcpy --show-touches --video-encoder=fail

To prevent this problem, ensure that the clean up thread is not
interrupted until the clean up process is started.

Refs #5601 <#5601>
PR #5613 <#5613>
If available, start the cleanup process in a new session to reduce the
likelihood of it being terminated along with the scrcpy server process
on some devices.

The binaries setsid and nohup are often available, but it is not
guaranteed.

Refs #5601 <#5601>
PR #5613 <#5613>
@rom1v rom1v merged commit c59a3c3 into dev Dec 8, 2024
@yume-chan
Copy link
Contributor

yume-chan commented Dec 9, 2024

My project is using Os.setsid method, it should do the same thing as setsid executable.

To my knowledge, on most devices, using setsid in a child process keeps it alive after unplugging USB cable, but they will still be killed when turning off USB Debugging/Wireless Debugging (when Wi-Fi is disconnected).

But some device still kills the process when unplugging USB cable, like the original issue reported.

Shizuku is using daemon, and the behavior is same.

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 9, 2024

My project is using Os.setsid method, it should do the same thing as setsid executable.

Ah yes, I will do that instead. I didn't know setsid() was exposed in Os. 👍

rom1v added a commit that referenced this pull request Dec 9, 2024
Reimplement c59a3c3 using Os.setsid().

Refs #5613 comment <#5613 (comment)>

Suggested-by: Simon Chan <[email protected]>
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