From 628275f6f982a1e269a23381a2dad69a4e440d97 Mon Sep 17 00:00:00 2001 From: Sean McBride Date: Thu, 30 Jul 2020 12:39:39 -0400 Subject: [PATCH] docs: Associate FIXMEs and TODOs with issues --- runtime/include/deque.h | 2 +- runtime/include/libuv_callbacks.h | 2 +- runtime/include/module.h | 3 --- runtime/include/sandbox.h | 2 +- runtime/include/types.h | 2 +- runtime/src/global_request_scheduler_minheap.c | 2 +- runtime/src/http_parser_settings.c | 7 +------ runtime/src/libc/uvio.c | 2 +- runtime/src/local_runqueue_minheap.c | 4 ++-- runtime/src/memory/64bit_nix.c | 4 ++-- runtime/src/memory/common.c | 6 +++--- runtime/src/module.c | 4 ++-- runtime/src/runtime.c | 2 +- runtime/src/sandbox.c | 8 +++++--- runtime/src/software_interrupt.c | 4 ++-- runtime/src/worker_thread.c | 11 +++++------ 16 files changed, 29 insertions(+), 36 deletions(-) diff --git a/runtime/include/deque.h b/runtime/include/deque.h index a49cfc0..621689b 100644 --- a/runtime/include/deque.h +++ b/runtime/include/deque.h @@ -20,7 +20,7 @@ * https://www.di.ens.fr/~zappa/readings/ppopp13.pdf */ -/* TODO: Implement the ability to dynamically resize! */ +/* TODO: Implement the ability to dynamically resize! Issue #89 */ #define DEQUE_MAX_SZ (1 << 23) #define DEQUE_PROTOTYPE(name, type) \ diff --git a/runtime/include/libuv_callbacks.h b/runtime/include/libuv_callbacks.h index ad47a9e..1f96615 100644 --- a/runtime/include/libuv_callbacks.h +++ b/runtime/include/libuv_callbacks.h @@ -16,7 +16,7 @@ * @param buffer unused * * FIXME: is there some weird edge case where a UNICODE character might be split between reads? Do we care? - * Called after libuv has read a chunk of data + * Called after libuv has read a chunk of data. Issue #100 */ static inline void libuv_callbacks_on_read_parse_http_request(uv_stream_t *stream, ssize_t number_read, const uv_buf_t *buffer) diff --git a/runtime/include/module.h b/runtime/include/module.h index 2f1d239..a2dae85 100644 --- a/runtime/include/module.h +++ b/runtime/include/module.h @@ -47,9 +47,6 @@ struct module { * so, using direct epoll for accepting connections. */ - // TODO: Should this be removed? - // uv_handle_t srvuv; - unsigned long max_request_size; char request_headers[HTTP_MAX_HEADER_COUNT][HTTP_MAX_HEADER_LENGTH]; int request_header_count; diff --git a/runtime/include/sandbox.h b/runtime/include/sandbox.h index 125d2c8..6b706f4 100644 --- a/runtime/include/sandbox.h +++ b/runtime/include/sandbox.h @@ -286,7 +286,7 @@ static inline void sandbox_close_file_descriptor(struct sandbox *sandbox, int io_handle_index) { if (io_handle_index >= SANDBOX_MAX_IO_HANDLE_COUNT || io_handle_index < 0) return; - /* TODO: Do we actually need to call some sort of close function here? */ + /* TODO: Do we actually need to call some sort of close function here? Issue #90 */ sandbox->io_handles[io_handle_index].file_descriptor = -1; } diff --git a/runtime/include/types.h b/runtime/include/types.h index 5b30b24..ba1cbb2 100644 --- a/runtime/include/types.h +++ b/runtime/include/types.h @@ -21,7 +21,7 @@ #define PAGE_ALIGNED __attribute__((aligned(PAGE_SIZE))) #define WEAK __attribute__((weak)) -/* FIXME: per-module configuration? */ +/* FIXME: per-module configuration? Issue #101 */ #define WASM_PAGE_SIZE (1024 * 64) /* 64KB */ #define WASM_START_PAGES (1 << 8) /* 16MB */ #define WASM_MAX_PAGES (1 << 15) /* 4GB */ diff --git a/runtime/src/global_request_scheduler_minheap.c b/runtime/src/global_request_scheduler_minheap.c index 6a96979..ef7e8c6 100644 --- a/runtime/src/global_request_scheduler_minheap.c +++ b/runtime/src/global_request_scheduler_minheap.c @@ -21,7 +21,7 @@ global_request_scheduler_minheap_add(void *sandbox_request) #endif int return_code = priority_queue_enqueue(&global_request_scheduler_minheap, sandbox_request); - /* TODO: Propagate -1 to caller */ + /* TODO: Propagate -1 to caller. Issue #91 */ if (return_code == -ENOSPC) panic("Request Queue is full\n"); return sandbox_request; } diff --git a/runtime/src/http_parser_settings.c b/runtime/src/http_parser_settings.c index 5aa63f8..eb59919 100644 --- a/runtime/src/http_parser_settings.c +++ b/runtime/src/http_parser_settings.c @@ -15,8 +15,6 @@ static http_parser_settings runtime_http_parser_settings; /** * http-parser data callback called when a URL is called * Sanity check to make sure that the path matches the name of the module - * TODO: Why does this not fail this assertion? To execute fibonacci, I just request localhost:10000, not - *localhost:10000/fibonacci * @param parser * @param at the start of the URL * @param length the length of the URL @@ -49,11 +47,8 @@ http_parser_settings_on_message_begin(http_parser *parser) /** * http-parser callback called when a header field is parsed - * Sets the key value of the latest header - * on a new header if last_was_value is true + * Sets the key value of the latest header on a new header if last_was_value is true * updating an existing header if last_was_value is false - * TODO: Is this logic correct? What is the relationship between fields and values? Is overwrite the correct logic if - *on_header executes twice in a row? * @param parser * @param at start address of the header field * @param length length of the header field diff --git a/runtime/src/libc/uvio.c b/runtime/src/libc/uvio.c index f96e934..a48d94d 100644 --- a/runtime/src/libc/uvio.c +++ b/runtime/src/libc/uvio.c @@ -478,7 +478,7 @@ wasm_ioctl(int32_t file_descriptor, int32_t request, int32_t data_offet) { // int d = current_sandbox_get_file_descriptor(file_descriptor); // musl libc does some ioctls to stdout, so just allow these to silently go through - // FIXME: The above is idiotic + // FIXME: The above is idiotic. Issue #102. // assert(d == 1); return 0; } diff --git a/runtime/src/local_runqueue_minheap.c b/runtime/src/local_runqueue_minheap.c index fdd767d..09c6497 100644 --- a/runtime/src/local_runqueue_minheap.c +++ b/runtime/src/local_runqueue_minheap.c @@ -32,7 +32,7 @@ local_runqueue_minheap_add(struct sandbox *sandbox) assert(!software_interrupt_is_enabled()); int return_code = priority_queue_enqueue(&local_runqueue_minheap, sandbox); - /* TODO: propagate RC to caller */ + /* TODO: propagate RC to caller. Issue #92 */ if (return_code == -ENOSPC) panic("Thread Runqueue is full!\n"); } @@ -173,7 +173,7 @@ local_runqueue_minheap_preempt(ucontext_t *user_context) /* * Restore the context of this new sandbox * user-level context switch state, so do not enable software interrupts. - * TODO: Review the interrupt logic here + * TODO: Review the interrupt logic here. Issue #63 */ arch_context_restore(&user_context->uc_mcontext, &next_sandbox->ctxt); should_enable_software_interrupt = false; diff --git a/runtime/src/memory/64bit_nix.c b/runtime/src/memory/64bit_nix.c index fb3bfd4..c0babc5 100644 --- a/runtime/src/memory/64bit_nix.c +++ b/runtime/src/memory/64bit_nix.c @@ -28,7 +28,7 @@ expand_memory(void) { struct sandbox *sandbox = current_sandbox_get(); - // max_pages = 0 => no limit: FIXME + // FIXME: max_pages = 0 => no limit. Issue #103. assert((sandbox->sandbox_size + local_sandbox_context_cache.linear_memory_size) / WASM_PAGE_SIZE < WASM_MAX_PAGES); // Remap the relevant wasm page to readable @@ -38,7 +38,7 @@ expand_memory(void) void *map_result = mmap(page_address, WASM_PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); - // TODO: Refactor to return RC signifying out-of-mem to caller + // TODO: Refactor to return RC signifying out-of-mem to caller. Issue #96. if (map_result == MAP_FAILED) panic("Mapping of new memory failed"); if (local_sandbox_context_cache.linear_memory_size > sandbox->linear_memory_max_size) panic("expand_memory - Out of Memory!\n"); diff --git a/runtime/src/memory/common.c b/runtime/src/memory/common.c index c27dc92..24823eb 100644 --- a/runtime/src/memory/common.c +++ b/runtime/src/memory/common.c @@ -11,7 +11,7 @@ initialize_region(uint32_t offset, uint32_t data_count, char *data) assert(local_sandbox_context_cache.linear_memory_size >= data_count); assert(offset < local_sandbox_context_cache.linear_memory_size - data_count); - /* FIXME: Hack around segmented and unsegmented access */ + /* FIXME: Hack around segmented and unsegmented access Issue #104 */ memcpy(get_memory_ptr_for_runtime(offset, data_count), data, data_count); } @@ -21,7 +21,7 @@ add_function_to_table(uint32_t idx, uint32_t type_id, char *pointer) assert(idx < INDIRECT_TABLE_SIZE); assert(local_sandbox_context_cache.module_indirect_table != NULL); - /* TODO: atomic for multiple concurrent invocations? */ + /* TODO: atomic for multiple concurrent invocations? Issue #97 */ if (local_sandbox_context_cache.module_indirect_table[idx].type_id == type_id && local_sandbox_context_cache.module_indirect_table[idx].func_pointer == pointer) return; @@ -35,5 +35,5 @@ add_function_to_table(uint32_t idx, uint32_t type_id, char *pointer) WEAK void populate_globals() { - assert(0); /* FIXME: is this used in WASM as dynamic modules? */ + assert(0); /* FIXME: is this used in WASM as dynamic modules? Issue #105. */ } diff --git a/runtime/src/module.c b/runtime/src/module.c index a20f0d3..41505ff 100644 --- a/runtime/src/module.c +++ b/runtime/src/module.c @@ -71,7 +71,8 @@ module_listen(struct module *module) * Closes the socket and dynamic library, and then frees the module * Returns harmlessly if there are outstanding references * - * TODO: Untested Functionality. Unsure if this will work + * TODO: Untested Functionality. Unsure if this will work. Also, what about the module database? Do we + * need to do any cleanup there? Issue #17 * @param module - the module to teardown */ void @@ -83,7 +84,6 @@ module_free(struct module *module) /* Do not free if we still have oustanding references */ if (module->reference_count) return; - /* TODO: What about the module database? Do we need to do any cleanup there? */ close(module->socket_descriptor); dlclose(module->dynamic_library_handle); diff --git a/runtime/src/runtime.c b/runtime/src/runtime.c index c4a1291..5bb47fe 100644 --- a/runtime/src/runtime.c +++ b/runtime/src/runtime.c @@ -33,7 +33,7 @@ runtime_initialize(void) assert(runtime_epoll_file_descriptor >= 0); /* Allocate and Initialize the global deque - TODO: Improve to expose variant as a config + TODO: Improve to expose variant as a config #Issue 93 */ // global_request_scheduler_deque_initialize(); global_request_scheduler_minheap_initialize(); diff --git a/runtime/src/sandbox.c b/runtime/src/sandbox.c index c834e0a..9769218 100644 --- a/runtime/src/sandbox.c +++ b/runtime/src/sandbox.c @@ -364,8 +364,10 @@ sandbox_allocate_memory(struct module *module) struct sandbox *sandbox = NULL; unsigned long sandbox_size = sizeof(struct sandbox) + module->max_request_or_response_size; - /* Control information should be page-aligned - TODO: Should I use round_up_to_page when setting sandbox_page? */ + /* + * Control information should be page-aligned + * TODO: Should I use round_up_to_page when setting sandbox_page? Issue #50 + */ assert(round_up_to_page(sandbox_size) == sandbox_size); /* At an address of the system's choosing, allocate the memory, marking it as inaccessible */ @@ -688,7 +690,7 @@ sandbox_set_as_returned(struct sandbox *sandbox, sandbox_state_t last_state) * Unmaps linear memory, removes from the runqueue (if on it), and adds to the completion queue * Because the stack is still in use, freeing the stack is deferred until later * - * TODO: Is the sandbox adding itself to the completion queue here? Is this a problem? + * TODO: Is the sandbox adding itself to the completion queue here? Is this a problem? Issue #94 * * @param sandbox the sandbox erroring out * @param last_state the state the sandbox is transitioning from. This is expressed as a constant to diff --git a/runtime/src/software_interrupt.c b/runtime/src/software_interrupt.c index 07642c1..6156c8a 100644 --- a/runtime/src/software_interrupt.c +++ b/runtime/src/software_interrupt.c @@ -87,7 +87,7 @@ sigalrm_handler(siginfo_t *signal_info, ucontext_t *user_context, struct sandbox * worker thread to run the main loop until it loads a new sandbox. * * TODO: Consider if this should be an invarient and the worker thread should disable software - * interrupts when doing this work. + * interrupts when doing this work. Issue #95 */ if (!current_sandbox) return; @@ -97,7 +97,7 @@ sigalrm_handler(siginfo_t *signal_info, ucontext_t *user_context, struct sandbox * about to switch to a new sandbox. * * TODO: Consider if this should be an invarient and the worker thread should disable software - * interrupts when doing this work. + * interrupts when doing this work. Issue #95 with above */ if (current_sandbox->state == SANDBOX_RETURNED) return; diff --git a/runtime/src/worker_thread.c b/runtime/src/worker_thread.c index 325a7a7..f05d3b2 100644 --- a/runtime/src/worker_thread.c +++ b/runtime/src/worker_thread.c @@ -62,8 +62,6 @@ worker_thread_transition_exiting_sandbox(struct sandbox *exiting_sandbox) /** * @brief Switches to the next sandbox, placing the current sandbox on the completion queue if in SANDBOX_RETURNED state * @param next_sandbox The Sandbox Context to switch to - * - * TODO: Confirm that this can gracefully resume sandboxes in a PREEMPTED state */ static inline void worker_thread_switch_to_sandbox(struct sandbox *next_sandbox) @@ -192,9 +190,11 @@ worker_thread_process_io(void) { #ifdef USE_HTTP_UVIO #ifdef USE_HTTP_SYNC - /* realistically, we're processing all async I/O on this core when a sandbox blocks on http processing, - * not great! if there is a way (TODO), perhaps RUN_ONCE and check if your I/O is processed, if yes, - * return else do async block! */ + /* + * TODO: realistically, we're processing all async I/O on this core when a sandbox blocks on http processing, + * not great! if there is a way, perhaps RUN_ONCE and check if your I/O is processed, if yes, + * return else do async block! Issue #98 + */ uv_run(worker_thread_get_libuv_handle(), UV_RUN_DEFAULT); #else /* USE_HTTP_SYNC */ worker_thread_block_current_sandbox(); @@ -280,7 +280,6 @@ worker_thread_main(void *return_code) * Called when the function in the sandbox exits * Removes the standbox from the thread-local runqueue, sets its state to SANDBOX_RETURNED, * releases the linear memory, and then returns to the base context - * TODO: Consider moving this to a future current_sandbox file. This has thus far proven difficult to move */ __attribute__((noreturn)) void worker_thread_on_sandbox_exit(struct sandbox *exiting_sandbox)