From 110dfa44a4ffb008dd34459778e759de600f3795 Mon Sep 17 00:00:00 2001 From: Brendan Hansen Date: Fri, 3 Dec 2021 15:29:37 -0600 Subject: [PATCH] bugfixes with windows process creation --- core/alloc/heap.onyx | 2 ++ core/runtime/onyx_run.onyx | 5 +-- include/small_windows.h | 10 ++++++ scripts/run_tests.onyx | 4 ++- src/wasm_runtime.c | 74 ++++++++++++++++++++++++++++---------- 5 files changed, 74 insertions(+), 21 deletions(-) diff --git a/core/alloc/heap.onyx b/core/alloc/heap.onyx index f8e1c8bf..f475e03d 100644 --- a/core/alloc/heap.onyx +++ b/core/alloc/heap.onyx @@ -238,6 +238,8 @@ get_freed_size :: () => { hb_ptr.size |= Allocated_Flag; new_ptr := heap_alloc(new_size_, align); + #if runtime.Multi_Threading_Enabled do sync.mutex_lock(^heap_mutex); + memory_copy(new_ptr, ptr, old_size - sizeof heap_block); heap_free(ptr); return new_ptr; diff --git a/core/runtime/onyx_run.onyx b/core/runtime/onyx_run.onyx index 48ab62fb..2faeea00 100644 --- a/core/runtime/onyx_run.onyx +++ b/core/runtime/onyx_run.onyx @@ -23,11 +23,12 @@ use package wasi Success :: 0x00; FailedToRun :: 0x01; Error :: 0x02; + InternalErr :: 0x03; } __process_spawn :: (path: str, args: [] str, non_blocking_io: bool) -> io.Process.Handle #foreign "env" "process_spawn" --- -__process_read :: (handle: io.Process.Handle, buffer: [] u8) -> u32 #foreign "env" "process_read" --- -__process_write :: (handle: io.Process.Handle, buffer: [] u8) -> u32 #foreign "env" "process_write" --- +__process_read :: (handle: io.Process.Handle, buffer: [] u8) -> i32 #foreign "env" "process_read" --- +__process_write :: (handle: io.Process.Handle, buffer: [] u8) -> i32 #foreign "env" "process_write" --- __process_kill :: (handle: io.Process.Handle) -> bool #foreign "env" "process_kill" --- __process_wait :: (handle: io.Process.Handle) -> ProcessResult #foreign "env" "process_wait" --- __process_destroy :: (handle: io.Process.Handle) -> void #foreign "env" "process_destroy" --- \ No newline at end of file diff --git a/include/small_windows.h b/include/small_windows.h index 0950b981..276ea408 100644 --- a/include/small_windows.h +++ b/include/small_windows.h @@ -312,6 +312,16 @@ GB_DLL_IMPORT BOOL WINAPI CreatePipe (HANDLE *hReadPipe, HANDLE *hWri GB_DLL_IMPORT BOOL WINAPI TerminateProcess (HANDLE hProcess, UINT uExitCode); GB_DLL_IMPORT BOOL WINAPI SetHandleInformation(HANDLE hObject, DWORD dwMask, DWORD dwFlags); +uintptr_t _beginthreadex( // NATIVE CODE + void *security, + unsigned stack_size, + unsigned ( __stdcall *start_address )( void * ), + void *arglist, + unsigned initflag, + unsigned *thrdaddr +); + + GB_DLL_IMPORT BOOL WINAPI GetLogicalProcessorInformation(SYSTEM_LOGICAL_PROCESSOR_INFORMATION *buffer, DWORD *return_length); GB_DLL_IMPORT DWORD_PTR WINAPI SetThreadAffinityMask(HANDLE thread, DWORD_PTR check_mask); GB_DLL_IMPORT HANDLE WINAPI GetCurrentThread(void); diff --git a/scripts/run_tests.onyx b/scripts/run_tests.onyx index 40bdffbd..c029768b 100644 --- a/scripts/run_tests.onyx +++ b/scripts/run_tests.onyx @@ -132,7 +132,7 @@ test_cases :: (cases) => { if exit := io.process_wait(^proc); exit != .Success { // Error running the test case - print_color(.Red, "[{}] Error compiling test case {}.\n{}", context.thread_id, it.source_file, output); + print_color(.Red, "[{}] Error '{}' in test case {}.\n{}", context.thread_id, exit, it.source_file, output); at_least_one_test_failed = true; continue; } @@ -143,6 +143,8 @@ test_cases :: (cases) => { if output != expected_output { print_color(.Red, "[{}] Output did not match for {}.\n", context.thread_id, it.source_file); + printf("Expected:\n{}\n", expected_output); + printf("Got:\n{}\n", output); at_least_one_test_failed = true; } } diff --git a/src/wasm_runtime.c b/src/wasm_runtime.c index fc9424f4..8d22038d 100644 --- a/src/wasm_runtime.c +++ b/src/wasm_runtime.c @@ -147,7 +147,8 @@ WASM_INTEROP(onyx_spawn_thread_impl) { #endif #ifdef _BH_WINDOWS - thread->thread_handle = CreateThread(NULL, 0, onyx_run_thread, thread, 0, &thread->thread_id); + // thread->thread_handle = CreateThread(NULL, 0, onyx_run_thread, thread, 0, &thread->thread_id); + thread->thread_handle = (HANDLE) _beginthreadex(NULL, 0, onyx_run_thread, thread, 0, &thread->thread_id); #endif results->data[0] = WASM_I32_VAL(1); @@ -166,6 +167,7 @@ WASM_INTEROP(onyx_kill_thread_impl) { #ifdef _BH_WINDOWS TerminateThread(thread->thread_handle, 0); + CloseHandle(thread->thread_handle); #endif bh_arr_deleten(threads, i, 1); @@ -307,20 +309,30 @@ WASM_INTEROP(onyx_process_spawn_impl) { success = success && CreatePipe(&process->host_to_proc_read, &process->host_to_proc_write, &saAttr, 4096); success = success && CreatePipe(&process->proc_to_host_read, &process->proc_to_host_write, &saAttr, 4096); if (!success) { + // printf("FAILED TO CREATE PIPES: %d\n", GetLastError()); wasm_val_init_ptr(&results->data[0], NULL); // Failed to run @LEAK return NULL; } - SetHandleInformation(process->proc_to_host_read, 1 /* HANDLE_FLAG_INHERIT */, 0); - SetHandleInformation(process->host_to_proc_write, 1 /* HANDLE_FLAG_INHERIT */, 0); + success = SetHandleInformation(process->proc_to_host_read, 1 /* HANDLE_FLAG_INHERIT */, 0); + success = success && SetHandleInformation(process->host_to_proc_write, 1 /* HANDLE_FLAG_INHERIT */, 0); + if (!success) { + // printf("FAILED TO CONFIGURE PIPES: %d\n", GetLastError()); + wasm_val_init_ptr(&results->data[0], NULL); // Failed to run @LEAK + return NULL; + } startup.hStdInput = process->host_to_proc_read; startup.hStdOutput = process->proc_to_host_write; startup.hStdError = process->proc_to_host_write; + startup.dwFlags |= STARTF_USESTDHANDLES; - success = CreateProcessA(process_path, cmdLine, NULL, NULL, 1, 0, NULL, NULL, &startup, &process->proc_info); + memset(&process->proc_info, 0, sizeof process->proc_info); + + success = CreateProcessA(process_path, cmdLine, &saAttr, &saAttr, 1, 0, NULL, NULL, &startup, &process->proc_info); if (!success) { + // printf("FAILED TO CREATE PROCESS: %d\n", GetLastError()); wasm_val_init_ptr(&results->data[0], NULL); // Failed to run @LEAK return NULL; } @@ -352,7 +364,7 @@ WASM_INTEROP(onyx_process_read_impl) { #ifdef _BH_WINDOWS BOOL success = ReadFile(process->proc_to_host_read, buffer, output_len, &bytes_read, NULL); - if (!success) bytes_read = 0; + if (!success) bytes_read = 0; #endif results->data[0] = WASM_I32_VAL(bytes_read); @@ -415,21 +427,48 @@ WASM_INTEROP(onyx_process_wait_impl) { #ifdef _BH_LINUX i32 status; waitpid(process->pid, &status, 0); - close(process->proc_to_host[0]); - close(process->host_to_proc[1]); i32 exit_code = WEXITSTATUS(status); results->data[0] = WASM_I32_VAL(exit_code != 0 ? 2 : 0); #endif #ifdef _BH_WINDOWS - DWORD result = WaitForSingleObject(process->proc_info.hProcess, INFINITE); - CloseHandle(process->host_to_proc_write); - CloseHandle(process->proc_to_host_write); - CloseHandle(process->proc_to_host_read); - DWORD exitCode; - GetExitCodeProcess(process->proc_info.hProcess, &exitCode); + while (1) { + if (!WaitForSingleObject(process->proc_info.hProcess, INFINITE)) { + // HACK HACK HACK + DWORD error = GetLastError(); + if (error != 109 && error != 6) { + // printf("ERROR IN WAIT FOR SINGLE: %d\n", error); + results->data[0] = WASM_I32_VAL(1); + return NULL; + } + } + + if (!GetExitCodeProcess(process->proc_info.hProcess, &exitCode)) { + // HACK HACK HACK + // Apparently, I'm doing something wrong (maybe?) where the process handle becomes + // invalid and causes error 6 "invalid handle". So I think I can safely assume that + // if that is the case, then the process exited? probably successfuly? hopefully? + // Honestly I don't know and I can't find any documentation describing when a process + // handle goes invalid, other than after you close it explicitly. But in the run_tests + // script, I'm not calling either process_kill or process_destroy, which are the only + // other functions that close the process handle. So I'm left in the dark as to why this + // is happening, but oh well. This works for now. + // - brendanfh 2021/12/03 + if (GetLastError() == 6) { + exitCode = 0; + break; + } + + results->data[0] = WASM_I32_VAL(3); + return NULL; + } + + // 259 is STILL_ACTIVE (aka STATUS_PENDING), which means that the process has not yet exited + if (exitCode != 259) break; + } + results->data[0] = WASM_I32_VAL(exitCode != 0 ? 2 : 0); #endif @@ -443,16 +482,15 @@ WASM_INTEROP(onyx_process_destroy_impl) { } #ifdef _BH_LINUX - kill(process->pid, SIGKILL); close(process->proc_to_host[0]); close(process->host_to_proc[1]); #endif #ifdef _BH_WINDOWS - TerminateProcess(process->proc_info.hProcess, 1); - CloseHandle(process->host_to_proc_write); - CloseHandle(process->proc_to_host_write); - CloseHandle(process->proc_to_host_read); + if (!CloseHandle(process->proc_info.hThread) + || !CloseHandle(process->proc_info.hProcess)) { + // printf("ERROR CLOSING HANDLES: %d\n", GetLastError()); + } #endif bh_free(global_heap_allocator, process); -- 2.25.1