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

Missing call of CloseHandle() in process.c (after successful CreateProcessW()) #4973

Closed
MarkusSchreiner opened this issue Jun 6, 2024 · 2 comments

Comments

@MarkusSchreiner
Copy link

Environment

  • OS: Windows 10
  • scrcpy version: 2.4

Describe the bug
A systematic Handle Leak is observed during debug-sessions.

Our analysis has shown, that in file:
scrcpy/app/src/sys/win/process.c

after a successful call of
BOOL ok = CreateProcessW(NULL, wide, NULL, NULL, bInheritHandles, dwCreationFlags, NULL, NULL, &si.StartupInfo, &pi);

the (nominal) call to
CloseHandle(pi.hThread);

is missing.

Please refer to the official Microsoft documentation
https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw

Quote:
"Handles in PROCESS_INFORMATION must be closed with CloseHandle when they are no longer needed."

@rom1v
Copy link
Collaborator

rom1v commented Jun 6, 2024

Thank you for the report.

Could you confirm that this fix is sufficient:

diff --git a/app/src/sys/win/process.c b/app/src/sys/win/process.c
index 6e9da09c9..6ae33d86d 100644
--- a/app/src/sys/win/process.c
+++ b/app/src/sys/win/process.c
@@ -176,6 +176,8 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle, unsigned flags,
         free(lpAttributeList);
     }
 
+    CloseHandle(pi.hThread);
+
     // These handles are used by the child process, close them for this process
     if (pin) {
         CloseHandle(stdin_read_handle);

In particular, that it is correct to close pi.hThread immediately? (Otherwise I will test later when I have access to a Windows machine)

@MarkusSchreiner
Copy link
Author

Yes, this seems ok.
Thanks for the quick response !

rom1v added a commit that referenced this issue Jun 9, 2024
@rom1v rom1v closed this as completed Jun 9, 2024
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

2 participants