From 4838dda59a7b0ddf2172f54810f46db1124c23f7 Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Tue, 14 Jan 2020 19:58:46 +0200 Subject: [PATCH] compiler: make [live] fn unlock its mutex on early return --- examples/hot_reload/bounce.v | 5 ++-- examples/hot_reload/graph.v | 5 +++- vlib/compiler/fn.v | 56 +++++++++++++++++++++++++++++------- vlib/compiler/gen_c.v | 5 ++++ vlib/compiler/live.v | 8 ++++-- vlib/compiler/main.v | 4 +-- 6 files changed, 64 insertions(+), 19 deletions(-) diff --git a/examples/hot_reload/bounce.v b/examples/hot_reload/bounce.v index 8833d033b0..9bea754ddd 100644 --- a/examples/hot_reload/bounce.v +++ b/examples/hot_reload/bounce.v @@ -56,6 +56,9 @@ fn main() { println('Starting the game loop...') go game.run() for { + if window.should_close() { + break + } gl.clear() gl.clear_color(255, 255, 255, 255) game.draw() @@ -101,5 +104,3 @@ fn (game mut Game) run() { time.sleep_ms(17) } } - - diff --git a/examples/hot_reload/graph.v b/examples/hot_reload/graph.v index dfd5d2e08b..0fd3d8c1cb 100644 --- a/examples/hot_reload/graph.v +++ b/examples/hot_reload/graph.v @@ -30,7 +30,10 @@ fn main() { }) } go update() // update the scene in the background in case the window isn't focused - for { + for { + if ctx.gg.window.should_close() { + break + } gg.clear(gx.White) ctx.draw() ctx.gg.render() diff --git a/vlib/compiler/fn.v b/vlib/compiler/fn.v index 20cba0b0a5..4d33ab62f5 100644 --- a/vlib/compiler/fn.v +++ b/vlib/compiler/fn.v @@ -225,10 +225,8 @@ fn (p mut Parser) fn_decl() { is_deprecated: p.attr == 'deprecated' comptime_define: if p.attr.starts_with('if ') { p.attr[3..] } else { '' } } - is_live := p.attr == 'live' && !p.pref.is_so && p.pref.is_live - if p.attr == 'live' && p.first_pass() && !p.pref.is_live && !p.pref.is_so { - println('INFO: run `v -live program.v` if you want to use [live] functions') - } + is_live := p.pref.is_live && p.attr == 'live' + is_solive := p.pref.is_solive && p.attr == 'live' if is_pub { p.next() p.fspace() @@ -488,6 +486,7 @@ fn (p mut Parser) fn_decl() { if is_c { p.fgen_nl() } + // Register the method if receiver_typ != '' { mut receiver_t := p.table.find_type(receiver_typ) @@ -509,6 +508,11 @@ fn (p mut Parser) fn_decl() { // '$p.file_name') p.table.register_fn(f) } + + if p.first_pass() && p.attr == 'live' && !(is_live || is_solive) { + println('INFO: run `v -live $p.v.dir `, if you want to use [live] function $f.name .') + } + if p.is_vh || p.first_pass() || is_live || is_fn_header || skip_main_in_test { // First pass? Skip the body for now // Look for generic calls. @@ -532,10 +536,34 @@ fn (p mut Parser) fn_decl() { } return } - if p.attr == 'live' && p.pref.is_so { - // p.genln('// live_function body start') - p.genln('pthread_mutex_lock(&live_fn_mutex);') + + if is_solive { + // Live functions are protected by a mutex, because otherwise they + // can be changed by the live reload thread, *while* they are + // running, with unpredictable results (usually just crashing). + // For this purpose, the actual body of the live function, + // is put under a non publicly accessible function, that is prefixed + // with 'impl_live_' . + // The live function just calls its implementation dual, while ensuring + // that the call is wrapped by the mutex lock & unlock calls. + // Adding the mutex lock/unlock inside the body of the implementation + // function is not reliable, because the implementation function can do + // an early exit, which will leave the mutex locked. + function_args := f.str_args_without_types(p.table) + mut live_fncall := 'impl_live_${fn_name_cgen}(${function_args});' + mut live_fnreturn := '' + if typ != 'void' { + live_fncall = '$typ res = $live_fncall' + live_fnreturn = 'return res;' + } + p.genln(' pthread_mutex_lock(&live_fn_mutex);') + p.genln(' $live_fncall') + p.genln(' pthread_mutex_unlock(&live_fn_mutex);') + p.genln(' $live_fnreturn') + p.genln('}') + p.genln('$typ impl_live_${fn_name_cgen} ($str_args){') } + if f.name in ['main__main', 'main', 'WinMain'] { if p.pref.is_test { p.error_with_token_index('tests cannot have function `main`', f.fn_name_token_idx) @@ -569,10 +597,6 @@ fn (p mut Parser) fn_decl() { if typ != 'void' && !p.returns { p.error_with_token_index('$f.name must return "$typ"', f.fn_name_token_idx) } - if p.attr == 'live' && p.pref.is_so { - // p.genln('// live_function body end') - p.genln('pthread_mutex_unlock(&live_fn_mutex);') - } if p.pref.x64 && f.name == 'main__main' && !p.first_pass() { //p.x64.gen_exit() } @@ -1611,6 +1635,16 @@ fn (f &Fn) str_args(table &Table) string { return s } + +// f.args => "a, b, c" +fn (f &Fn) str_args_without_types(table &Table) string { + mut res := []string + for arg in f.args { + res << arg.name + } + return res.join(', ') +} + // find local function variable with closest name to `name` fn (p &Parser) find_misspelled_local_var(name string, min_match f32) string { mut closest := f32(0) diff --git a/vlib/compiler/gen_c.v b/vlib/compiler/gen_c.v index f7d3988084..a86c082e2d 100644 --- a/vlib/compiler/gen_c.v +++ b/vlib/compiler/gen_c.v @@ -54,6 +54,11 @@ fn (p mut Parser) gen_fn_decl(f Fn, typ, str_args string) { dll_export_linkage := if p.pref.ccompiler == 'msvc' && p.attr == 'live' && p.pref.is_so { '__declspec(dllexport) ' } else if p.attr == 'inline' { 'static inline ' } else { '' } fn_name_cgen := p.table.fn_gen_name(f) // str_args := f.str_args(p.table) + + if p.attr == 'live' && p.pref.is_so { + // See fn.v for details about impl_live_ functions + p.genln('$typ impl_live_${fn_name_cgen} ($str_args);') + } p.genln('$dll_export_linkage$typ $fn_name_cgen ($str_args) {') } diff --git a/vlib/compiler/live.v b/vlib/compiler/live.v index 12408f0351..cf799a46e9 100644 --- a/vlib/compiler/live.v +++ b/vlib/compiler/live.v @@ -100,9 +100,11 @@ fn (v &V) generate_hot_reload_code() { } ticks := time.ticks() os.system(cmd_compile_shared_library) - diff := time.ticks() - ticks - println('compiling shared library took $diff ms') - println('=========\n') + if v.pref.is_verbose { + diff := time.ticks() - ticks + println('compiling shared library took $diff ms') + println('=========\n') + } cgen.genln(' void lfnmutex_print(char *s){ diff --git a/vlib/compiler/main.v b/vlib/compiler/main.v index 3930838585..f8049040d8 100644 --- a/vlib/compiler/main.v +++ b/vlib/compiler/main.v @@ -472,7 +472,7 @@ fn (v mut V) generate_init() { } ') } - if !v.pref.is_bare { + if !v.pref.is_bare && !v.pref.is_so { // vlib can't have `init_consts()` v.cgen.genln('void init() { #if VPREALLOC @@ -581,7 +581,7 @@ pub fn (v mut V) generate_main() { } v.gen_main_end('return g_test_fails > 0') } - else if v.table.main_exists() { + else if v.table.main_exists() && !v.pref.is_so { v.gen_main_start(true) cgen.genln(' main__main();') if !v.pref.is_bare {