Alternate Thread Creation API To Avoid Memory Leaks (#306)

* Alternate Thread Creation API To Avoid Memory Leaks

- Switched from CreateThread() to _beginthreadex() and ExitThread() to _endthreadex() in order to avoid potential leaks when linking with static CRT library.
- Addressed a variety of warnings that were being detected with static code analysis.

* Addressed Type Cast Warning

- Added explicit cast to the output of _beginthreadex() to avoid a compiler warning.

* Indentation Fix
This commit is contained in:
Bryan Berns 2018-05-23 19:53:36 -04:00 committed by Manoj Ampalam
parent 236b04b335
commit ec102dce28
11 changed files with 40 additions and 38 deletions

View File

@ -1,6 +1,6 @@
#pragma once
void fatal(const char *, ...);
void __declspec(noreturn) fatal(const char *, ...);
void error(const char *, ...);
void verbose(const char *, ...);
void debug(const char *, ...);

View File

@ -1072,8 +1072,14 @@ fileio_readlink(const char *path, char *buf, size_t bufsiz)
goto cleanup;
}
/* allocate the maximum possible size the reparse buffer size could be */
reparse_buffer = (PREPARSE_DATA_BUFFER_SYMLINK)malloc(MAXIMUM_REPARSE_DATA_BUFFER_SIZE);
if (reparse_buffer == NULL) {
errno = ENOMEM;
goto cleanup;
}
/* send a request to the file system to get the real path */
reparse_buffer = (PREPARSE_DATA_BUFFER_SYMLINK) malloc(MAXIMUM_REPARSE_DATA_BUFFER_SIZE);
DWORD dwBytesReturned = 0;
if (DeviceIoControl(handle, FSCTL_GET_REPARSE_POINT, NULL, 0,
(LPVOID) reparse_buffer, MAXIMUM_REPARSE_DATA_BUFFER_SIZE, &dwBytesReturned, 0) == 0) {

View File

@ -1,6 +0,0 @@
#ifndef Process_H
#define Process_H
/* Compatibility header to avoid lots of #ifdef _WIN32's in includes.h */
#endif

View File

@ -1034,8 +1034,8 @@ resolved_path_utf16(const char *input_path)
else
resolved_path = utf8_to_utf16(input_path);
if (resolved_path == NULL) {
errno = ENOMEM;
if (resolved_path == NULL) {
errno = ENOMEM;
return NULL;
}
@ -1052,10 +1052,10 @@ resolved_path_utf16(const char *input_path)
if (changed_req > 0) {
wchar_t * resolved_path_new = realloc(resolved_path,
(resolved_len + changed_req + 1) * sizeof(wchar_t));
if (resolved_path == NULL) {
debug3("%s: memory allocation failed.", __FUNCTION__);
free(resolved_path);
errno = ENOMEM;
if (resolved_path_new == NULL) {
debug3("%s: memory allocation failed.", __FUNCTION__);
free(resolved_path);
errno = ENOMEM;
return NULL;
}
else resolved_path = resolved_path_new;

View File

@ -38,6 +38,7 @@
#include <io.h>
#include <Shlobj.h>
#include <Sddl.h>
#include <process.h>
#include "misc_internal.h"
#include "inc\utf.h"
@ -790,7 +791,7 @@ SizeWindow(HANDLE hInput)
bSuccess = GetConsoleScreenBufferInfoEx(hInput, &consoleInfo);
}
DWORD WINAPI
unsigned __stdcall
MonitorChild(_In_ LPVOID lpParameter)
{
WaitForSingleObject(child, INFINITE);
@ -1009,7 +1010,7 @@ ProcessEvent(void *p)
return ERROR_SUCCESS;
}
DWORD WINAPI
unsigned __stdcall
ProcessEventQueue(LPVOID p)
{
while (1) {
@ -1107,7 +1108,7 @@ void FreeQueueEvent()
LeaveCriticalSection(&criticalSection);
}
DWORD WINAPI
unsigned __stdcall
ProcessPipes(LPVOID p)
{
BOOL ret;
@ -1293,7 +1294,7 @@ start_with_pty(wchar_t *command)
/* monitor child exist */
child = pi.hProcess;
monitor_thread = CreateThread(NULL, 0, MonitorChild, NULL, 0, NULL);
monitor_thread = (HANDLE) _beginthreadex(NULL, 0, MonitorChild, NULL, 0, NULL);
if (IS_INVALID_HANDLE(monitor_thread))
goto cleanup;
@ -1302,11 +1303,11 @@ start_with_pty(wchar_t *command)
initialize_keylen();
io_thread = CreateThread(NULL, 0, ProcessPipes, NULL, 0, NULL);
io_thread = (HANDLE) _beginthreadex(NULL, 0, ProcessPipes, NULL, 0, NULL);
if (IS_INVALID_HANDLE(io_thread))
goto cleanup;
ux_thread = CreateThread(NULL, 0, ProcessEventQueue, NULL, 0, NULL);
ux_thread = (HANDLE) _beginthreadex(NULL, 0, ProcessEventQueue, NULL, 0, NULL);
if (IS_INVALID_HANDLE(ux_thread))
goto cleanup;
@ -1348,7 +1349,6 @@ cleanup:
return child_exit_code;
}
/* shellhost.exe <cmdline to be executed with PTY support>*/
int
wmain(int ac, wchar_t **av)
{

View File

@ -1,4 +1,5 @@
#include <Windows.h>
#include <process.h>
/* child processes */
#define MAX_CHILDREN 512

View File

@ -43,7 +43,7 @@ typedef struct _wait_for_multiple_objects_struct {
}
wait_for_multiple_objects_struct;
DWORD WINAPI
unsigned __stdcall
wait_for_multiple_objects_thread(LPVOID lpParam)
{
wait_for_multiple_objects_struct *waitstruct =
@ -66,7 +66,7 @@ wait_for_multiple_objects_interrupter(_In_ ULONG_PTR dwParam)
* the alert prior to the thread running in which case it is acknowledged when
* the threads starts running instead of when it is waiting at
* WaitForMultipleObjectsEx */
ExitThread(0);
_endthreadex(0);
}
DWORD
@ -144,8 +144,8 @@ wait_for_multiple_objects_enhanced(_In_ DWORD nCount, _In_ const HANDLE *lpHand
wait_bins[bin].num_handles = min(nCount - handles_processed, bin_size);
/* create a thread for this bin */
if ((wait_bins[bin].thread_handle = CreateThread(NULL, 2048,
wait_for_multiple_objects_thread,
if ((wait_bins[bin].thread_handle = (HANDLE) _beginthreadex(NULL,
2048, wait_for_multiple_objects_thread,
(LPVOID) &(wait_bins[bin]), 0, NULL)) == NULL) {
goto cleanup;
}

View File

@ -40,6 +40,7 @@
*/
#include <Windows.h>
#include <process.h>
#include "w32fd.h"
#include "tncon.h"
#include "inc\utf.h"
@ -69,7 +70,7 @@ ReadAPCProc(_In_ ULONG_PTR dwParam)
}
/* Read worker thread */
static DWORD WINAPI
static unsigned __stdcall
ReadThread(_In_ LPVOID lpParameter)
{
int nBytesReturned = 0;
@ -154,10 +155,10 @@ syncio_initiate_read(struct w32_io* pio)
pio->read_details.buf_size = TERM_IO_BUF_SIZE;
}
read_thread = CreateThread(NULL, 0, ReadThread, pio, 0, NULL);
read_thread = (HANDLE) _beginthreadex(NULL, 0, ReadThread, pio, 0, NULL);
if (read_thread == NULL) {
errno = errno_from_Win32LastError();
debug3("TermRead initiate - ERROR CreateThread %d, io:%p", GetLastError(), pio);
debug3("TermRead initiate - ERROR _beginthreadex %d, io:%p", GetLastError(), pio);
return -1;
}
@ -185,7 +186,7 @@ WriteAPCProc(_In_ ULONG_PTR dwParam)
/* Write worker thread */
static DWORD WINAPI
static unsigned __stdcall
WriteThread(_In_ LPVOID lpParameter)
{
struct w32_io* pio = (struct w32_io*)lpParameter;
@ -231,10 +232,10 @@ syncio_initiate_write(struct w32_io* pio, DWORD num_bytes)
debug5("syncio_initiate_write initiate io:%p", pio);
memset(&(pio->sync_write_status), 0, sizeof(pio->sync_write_status));
pio->sync_write_status.to_transfer = num_bytes;
write_thread = CreateThread(NULL, 0, WriteThread, pio, 0, NULL);
write_thread = (HANDLE)_beginthreadex(NULL, 0, WriteThread, pio, 0, NULL);
if (write_thread == NULL) {
errno = errno_from_Win32LastError();
debug3("syncio_initiate_write initiate - ERROR CreateThread %d, io:%p", GetLastError(), pio);
debug3("syncio_initiate_write initiate - ERROR _beginthreadex %d, io:%p", GetLastError(), pio);
return -1;
}

View File

@ -1126,7 +1126,7 @@ fd_encode_state(const posix_spawn_file_actions_t *file_actions, HANDLE aux_h[])
char *buf, *encoded;
struct std_fd_state *std_fd_state;
struct inh_fd_state *c;
DWORD len_req;
DWORD len_req = 0;
BOOL b;
int i;
int fd_in = file_actions->stdio_redirect[STDIN_FILENO];
@ -1174,7 +1174,7 @@ static void
fd_decode_state(char* enc_buf)
{
char* buf;
DWORD req, skipped, out_flags;
DWORD req = 0, skipped, out_flags;
struct std_fd_state *std_fd_state;
struct inh_fd_state *c;
int num_inherited = 0;

View File

@ -142,7 +142,7 @@ syslog_file(int priority, const char *format, const char *formatBuffer)
GetCurrentProcessId(), st.wYear, st.wMonth, st.wDay, st.wHour, st.wMinute, st.wSecond,
st.wMilliseconds, formatBuffer);
if (r == -1) {
_write(logfd, "_snprintf_s failed.", 30);
_write(logfd, "_snprintf_s failed.", 20);
return;
}
msgbufTimestamp[strnlen(msgbufTimestamp, MSGBUFSIZ)] = '\0';

View File

@ -23,7 +23,7 @@ signal_test_send_apc(LPVOID lpParam)
return TRUE;
}
DWORD WINAPI
unsigned __stdcall
signal_test_set_event(LPVOID lpParam)
{
HANDLE hevent = (HANDLE)lpParam;
@ -32,7 +32,7 @@ signal_test_set_event(LPVOID lpParam)
return TRUE;
}
DWORD WINAPI
unsigned __stdcall
signal_create_abandoned_object(LPVOID lpParam)
{
*((HANDLE *)lpParam) = CreateMutex(NULL, TRUE, 0);
@ -65,7 +65,7 @@ signal_test_wait_for_multiple_objects()
/* create abandoned mutex */
HANDLE abandoned_mutux = NULL;
HANDLE mutex_thread = CreateThread(NULL, 0, signal_create_abandoned_object, &abandoned_mutux, 0, NULL);
HANDLE mutex_thread = (HANDLE) _beginthreadex(NULL, 0, signal_create_abandoned_object, &abandoned_mutux, 0, NULL);
WaitForSingleObject(mutex_thread, INFINITE);
CloseHandle(mutex_thread);
@ -137,7 +137,7 @@ signal_test_wait_for_multiple_objects()
for (int i = 0; i < objects_size; i++) ResetEvent(hObjects[i]);
for (int i = 0; i < objects_size; i++) {
CloseHandle(CreateThread(NULL, 0, signal_test_set_event, hObjects[i], 0, NULL));
CloseHandle((HANDLE) _beginthreadex(NULL, 0, signal_test_set_event, hObjects[i], 0, NULL));
DWORD ret = wait_for_multiple_objects_enhanced(objects_size, hObjects, 10000, FALSE);
ASSERT_INT_EQ(ret, i + WAIT_OBJECT_0_ENHANCED);
ResetEvent(hObjects[i]);