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

Inherit only specific handles on Windows #2783

Merged
merged 1 commit into from
Nov 15, 2021
Merged

Conversation

rom1v
Copy link
Collaborator

@rom1v rom1v commented Nov 14, 2021

Inherit only specific handles on Windows

To be able to communicate with a child process via stdin, stdout and
stderr, the CreateProcess() parameter bInheritHandles must be set to
TRUE. But this causes all handles to be inherited, including sockets.

As a result, the server socket was inherited by the process running adb
to execute the server on the device, so it could not be closed properly,
causing other scrcpy instances to fail.

To fix the issue, use an extended API to explicitly set the HANDLEs to
inherit:

Fixes #2779


Replace these binaries in your v1.20 release:

  • scrcpy.exe sha256:33a57452cbd58c689a30497c28e005111553e2ab21f6ddfee270767c29b5cceb
  • scrcpy-server sha256:b20aee4951f99b060c4a44000ba94de973f9604758ef62beb253b371aad3df34

To be able to communicate with a child process via stdin, stdout and
stderr, the CreateProcess() parameter bInheritHandles must be set to
TRUE. But this causes *all* handles to be inherited, including sockets.

As a result, the server socket was inherited by the process running adb
to execute the server on the device, so it could not be closed properly,
causing other scrcpy instances to fail.

To fix the issue, use an extended API to explicitly set the HANDLEs to
inherit:
 - <https://stackoverflow.com/a/28185363/1987178>
 - <https://devblogs.microsoft.com/oldnewthing/20111216-00/?p=8873>

Fixes #2779 <#2779>
@rom1v rom1v merged commit 1eaa73d into dev Nov 15, 2021
rom1v added a commit that referenced this pull request Nov 15, 2021
To be able to communicate with a child process via stdin, stdout and
stderr, the CreateProcess() parameter bInheritHandles must be set to
TRUE. But this causes *all* handles to be inherited, including sockets.

As a result, the server socket was inherited by the process running adb
to execute the server on the device, so it could not be closed properly,
causing other scrcpy instances to fail.

To fix the issue, use an extended API to explicitly set the HANDLEs to
inherit:
 - <https://stackoverflow.com/a/28185363/1987178>
 - <https://devblogs.microsoft.com/oldnewthing/20111216-00/?p=8873>

Fixes #2779 <#2779>
PR #2783 <#2783>
@rom1v
Copy link
Collaborator Author

rom1v commented Nov 15, 2021

Merged into dev: 9cb14b5

@rom1v rom1v mentioned this pull request Nov 18, 2021
rom1v added a commit that referenced this pull request Nov 26, 2021
If SOCK_CLOEXEC exists, then set the flag on socket creation.

Otherwise, use fcntl() (or SetHandleInformation() on Windows) to set the
flag afterwards.

This avoids the sockets to be inherited in child processes.

Refs #2783 <#2783>
rom1v added a commit that referenced this pull request Nov 26, 2021
If SOCK_CLOEXEC exists, then set the flag on socket creation.

Otherwise, use fcntl() (or SetHandleInformation() on Windows) to set the
flag afterwards.

This avoids the sockets to be inherited in child processes.

Refs #2783 <#2783>
@rom1v rom1v mentioned this pull request Nov 29, 2021
rom1v added a commit that referenced this pull request Nov 30, 2021
According to this bug report on Firefox:
<https://bugzilla.mozilla.org/show_bug.cgi?id=1460995>

> CreateProcess fails with ERROR_NO_SYSTEM_RESOURCES on Windows 7. It
> looks like the reason why is because PROC_THREAD_ATTRIBUTE_HANDLE_LIST
> doesn't like console handles.

To avoid the problem, do not pass console handles to
PROC_THREAD_ATTRIBUTE_HANDLE_LIST.

Refs #2783 <#2783>
Refs f801d8b
Fixes #2838 <#2838>
rom1v added a commit that referenced this pull request Dec 1, 2021
According to this bug report on Firefox:
<https://bugzilla.mozilla.org/show_bug.cgi?id=1460995>

> CreateProcess fails with ERROR_NO_SYSTEM_RESOURCES on Windows 7. It
> looks like the reason why is because PROC_THREAD_ATTRIBUTE_HANDLE_LIST
> doesn't like console handles.

To avoid the problem, do not pass console handles to
PROC_THREAD_ATTRIBUTE_HANDLE_LIST.

Refs #2783 <#2783>
Refs f801d8b
Fixes #2838 <#2838>
PR #2840 <#2840>
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.

None yet

1 participant