From d6f368a3f400fea3e89280262a8147e7ce5d855c Mon Sep 17 00:00:00 2001 From: dec05eba Date: Thu, 22 Aug 2019 00:59:49 +0200 Subject: Move thread work from compiler/parser to thread_work file, fix use after free bug in multithreaded parser allocator --- include/compiler.h | 9 ++---- include/defs.h | 1 - include/parser.h | 17 ------------ include/std/buffer.h | 2 +- include/std/thread.h | 2 +- include/std/thread_pool.h | 71 +++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 76 insertions(+), 26 deletions(-) create mode 100644 include/std/thread_pool.h (limited to 'include') diff --git a/include/compiler.h b/include/compiler.h index 08b74b8..ac50d28 100644 --- a/include/compiler.h +++ b/include/compiler.h @@ -5,7 +5,7 @@ #include "std/buffer.h" #include "std/buffer_view.h" #include "std/arena_allocator.h" -#include "std/thread.h" +#include "std/thread_pool.h" #include "compiler_options.h" #include "ast.h" #include "program.h" @@ -49,20 +49,17 @@ struct amal_compiler { ArenaAllocator allocator; Scope root_scope; Buffer/**/ parsers; - Buffer/**/ queued_files; HashMapType(BufferView, FileScopeReference*) file_scopes; - ParserThreadData *threads; - int usable_thread_count; bool started; - bool work_failed; amal_mutex mutex; - int generic_work_object_index; + amal_thread_pool stage_task_thread_pool; }; void amal_compiler_options_init(amal_compiler_options *self); /* If @options is NULL, then default values are used. + @options are copied. You should run @amal_program_deinit even @amal_compiler_load_file fails, to cleanup memory. This function creates a copy of @filepath so it doesn't have to survive longer than this function call. The file has to be in utf-8 format and it can optionally have utf-8 BOM. diff --git a/include/defs.h b/include/defs.h index 8cd9d39..c8db820 100644 --- a/include/defs.h +++ b/include/defs.h @@ -1,7 +1,6 @@ #ifndef AMALGAM_DEFS_H #define AMALGAM_DEFS_H -typedef struct ParserThreadData ParserThreadData; typedef struct amal_compiler amal_compiler; typedef struct Parser Parser; typedef struct Scope Scope; diff --git a/include/parser.h b/include/parser.h index 4427cc8..81895e1 100644 --- a/include/parser.h +++ b/include/parser.h @@ -16,18 +16,6 @@ #define PARSER_ERR -1 #define PARSER_UNEXPECTED_TOKEN -2 -typedef enum { - PARSER_THREAD_STATUS_NEW, - PARSER_THREAD_STATUS_IDLE, - PARSER_THREAD_STATUS_RUNNING -} ParserThreadStatus; - -struct ParserThreadData { - amal_thread thread; - ParserThreadStatus status; - ArenaAllocator allocator; -}; - typedef enum { ERROR_CONTEXT_NONE, ERROR_CONTEXT_RHS_STANDALONE, @@ -51,11 +39,6 @@ struct Parser { Bytecode bytecode; }; -CHECK_RESULT int parser_thread_data_init(ParserThreadData *self); -void parser_thread_data_deinit(ParserThreadData *self); -CHECK_RESULT int parser_thread_data_start(ParserThreadData *self, AmalThreadCallbackFunc callback_func, void *userdata); -CHECK_RESULT int parser_thread_data_join(ParserThreadData *self, void **result); - CHECK_RESULT int parser_init(Parser *self, amal_compiler *compiler, ArenaAllocator *allocator); /* diff --git a/include/std/buffer.h b/include/std/buffer.h index ac722b1..d194881 100644 --- a/include/std/buffer.h +++ b/include/std/buffer.h @@ -30,7 +30,7 @@ CHECK_RESULT int buffer_append_empty(Buffer *self, usize size); CHECK_RESULT int buffer_expand(Buffer *self, usize size); void* buffer_get(Buffer *self, usize index, usize type_size); CHECK_RESULT int buffer_pop(Buffer *self, void *data, usize size); -/* Set buffer size to 0, doesn't reallocate */ +/* Set buffer size to 0, doesn't change the capacity */ void buffer_clear(Buffer *self); void* buffer_begin(Buffer *self); void* buffer_end(Buffer *self); diff --git a/include/std/thread.h b/include/std/thread.h index 356ebf0..2765204 100644 --- a/include/std/thread.h +++ b/include/std/thread.h @@ -47,7 +47,7 @@ bool amal_thread_is_main(); /* Returns 0 if the number of usable threads is unknown */ int amal_get_usable_thread_count(); -void amal_mutex_init(amal_mutex *self); +CHECK_RESULT int amal_mutex_init(amal_mutex *self); void amal_mutex_deinit(amal_mutex *self); CHECK_RESULT int amal_mutex_lock(amal_mutex *self, const char *lock_identifier); /* Safe to call unlock when another thread owns the lock or if the lock is not locked */ diff --git a/include/std/thread_pool.h b/include/std/thread_pool.h new file mode 100644 index 0000000..f8acf83 --- /dev/null +++ b/include/std/thread_pool.h @@ -0,0 +1,71 @@ +#ifndef AMAL_THREAD_POOL_H +#define AMAL_THREAD_POOL_H + +#include "thread.h" +#include "buffer.h" +#include "buffer_view.h" + +/* + Return 0 if there is no error, otherwise return a non-0 value. + If the result is not 0, then all additional tasks will be stopped. +*/ +typedef int(*amal_thread_job_callback)(void *userdata); + +typedef enum { + THREAD_POOL_THREAD_STATUS_NEW, + THREAD_POOL_THREAD_STATUS_IDLE, + THREAD_POOL_THREAD_STATUS_RUNNING +} amal_thread_pool_thread_status; + +typedef struct { + amal_thread thread; + amal_thread_pool_thread_status status; +} amal_thread_pool_thread; + +typedef struct { + amal_thread_job_callback callback; + void *userdata; +} amal_thread_pool_task; + +typedef struct { + int num_threads; + amal_thread_pool_thread *threads; /* Size of @threads is @num_threads */ + amal_mutex task_select_mutex; + Buffer queued_tasks; + bool dead; + int num_finished_queued_tasks; +} amal_thread_pool; + +typedef struct { + amal_thread_pool *thread_pool; + amal_thread_pool_thread *thread_pool_thread; + amal_thread_job_callback callback; + void *userdata; +} amal_thread_pool_callback_data; + +/* + If @num_threads is 0, then the number of threads selected will the number + of physical threads, or 1 +*/ +CHECK_RESULT int thread_pool_init(amal_thread_pool *self, int num_threads); +void thread_pool_deinit(amal_thread_pool *self); +/* + Thread-safe. Will fail if one task has failed, in which case it can only be used + once @thread_pool_join_all_tasks has been called +*/ +CHECK_RESULT int thread_pool_add_task(amal_thread_pool *self, amal_thread_job_callback callback, void *userdata); +/* + Wait until all tasks have finished. Should only be called from one thread. + Returns true if all tasks finished without any issues (if @thread_pool_mark_dead was not called) +*/ +CHECK_RESULT bool thread_pool_join_all_tasks(amal_thread_pool *self); +/* + Stop tasks as soon as possible and stop dispatch of new tasks. + This can be used when work on one thread has failed and you want to + stop tasks on all threads to report errors and stop additional tasks. +*/ +void thread_pool_mark_dead(amal_thread_pool *self); + +BufferView/**/ thread_pool_get_threads(amal_thread_pool *self); + +#endif -- cgit v1.2.3