From 74363a481ed8edfef0d54349edbfbc748cf62101 Mon Sep 17 00:00:00 2001 From: Cesar Douady Date: Tue, 30 Apr 2024 11:41:17 +0200 Subject: [PATCH 1/9] improve ldebug -c + fix lack of dep on static dep --- Manifest | 1 + _lib/lmake_dbg.py | 6 +++--- src/autodep/clmake.cc | 1 + src/lmakeserver/cmd.cc | 15 +++++++------- src/lmakeserver/job.cc | 12 +++++------ src/lmakeserver/node.x.hh | 32 +++++++++++++++-------------- unit_tests/misc13.py | 42 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 78 insertions(+), 31 deletions(-) create mode 100644 unit_tests/misc13.py diff --git a/Manifest b/Manifest index 2d0821ea..1ec92719 100644 --- a/Manifest +++ b/Manifest @@ -192,6 +192,7 @@ unit_tests/manual_target.py unit_tests/misc1.py unit_tests/misc10.py unit_tests/misc11.py +unit_tests/misc13.py unit_tests/misc2.py unit_tests/misc3.py unit_tests/misc4.py diff --git a/_lib/lmake_dbg.py b/_lib/lmake_dbg.py index c67a8c87..a8dea3d1 100644 --- a/_lib/lmake_dbg.py +++ b/_lib/lmake_dbg.py @@ -85,15 +85,15 @@ def run_pdb(dbg_dir,redirected,func,*args,**kwds) : def run_vscode(dbg_dir,redirected,func,*args,**kwds) : import json import os + import signal + signal.signal(signal.SIGPIPE,signal.SIG_IGN) # vscode seems to be sensitive to SIGPIPE try : # Write python process information to vscode debug workspace to allow gdb to attache to it workspace = dbg_dir + '/vscode/ldebug.code-workspace' if os.path.exists(workspace) : data = json.load(open(workspace)) for elmt in data['launch']['configurations'] : - if elmt.get('type') == 'by-gdb' : - if 'processId' in elmt : elmt['processId'] = os.getpid() - if 'program' in elmt : elmt['program' ] = open('/proc/self/cmdline').read().split('\x00')[:-1] + if elmt.get('type')=='by-gdb' and 'processId' in elmt : elmt['processId'] = os.getpid() with open(workspace,'w') as out : json.dump(data,out,indent='\t') out.write('\n') diff --git a/src/autodep/clmake.cc b/src/autodep/clmake.cc index 016f44f2..0ed06ac8 100644 --- a/src/autodep/clmake.cc +++ b/src/autodep/clmake.cc @@ -248,6 +248,7 @@ static PyObject* get_autodep( PyObject* /*null*/ , PyObject* args , PyObject* kw char c = 0/*garbage*/ ; // we have a private Record with a private AutodepEnv, so we must go through the backdoor to alter the regular AutodepEnv int rc [[maybe_unused]] = ::readlinkat( Record::s_root_fd() , (PrivateAdminDir+"/backdoor/autodep"s ).c_str() , &c , 1 ) ; + if (rc<0) return None.to_py_boost() ; SWEAR( c=='0' || c=='1' , int(c) ) ; SWEAR( rc==1 , rc ) ; return Ptr(c!='0')->to_py_boost() ; diff --git a/src/lmakeserver/cmd.cc b/src/lmakeserver/cmd.cc index e8b6c3a8..ec4164ee 100644 --- a/src/lmakeserver/cmd.cc +++ b/src/lmakeserver/cmd.cc @@ -265,7 +265,7 @@ R"({ "type" : "by-gdb" , "request" : "attach" , "name" : "Attach C/C++" - , "program" : "" + , "program" : $interpreter , "cwd" : $g_root_dir , "processId" : 0 } @@ -285,10 +285,11 @@ R"({ /**/ append_to_string( extensions , '"',ext,'"' ) ; first = false ; } - res = ::regex_replace( res , ::regex("\\$extensions") , extensions ) ; - res = ::regex_replace( res , ::regex("\\$name" ) , mk_json_str( j->name() ) ) ; - res = ::regex_replace( res , ::regex("\\$g_root_dir") , mk_json_str( *g_root_dir ) ) ; - res = ::regex_replace( res , ::regex("\\$program" ) , mk_json_str(to_string(*g_root_dir,'/',dbg_dir,"/cmd")) ) ; + res = ::regex_replace( res , ::regex("\\$extensions" ) , extensions ) ; + res = ::regex_replace( res , ::regex("\\$name" ) , mk_json_str( j->name() ) ) ; + res = ::regex_replace( res , ::regex("\\$g_root_dir" ) , mk_json_str( *g_root_dir ) ) ; + res = ::regex_replace( res , ::regex("\\$program" ) , mk_json_str(to_string(*g_root_dir,'/',dbg_dir,"/cmd")) ) ; + res = ::regex_replace( res , ::regex("\\$interpreter") , mk_json_str(to_string(start.interpreter[0] )) ) ; // ::vmap_ss env = _mk_env(start.env,report_end.end.dynamic_env) ; size_t kw = 13/*SEQUENCE_ID*/ ; for( auto&& [k,v] : env ) if (k!="TMPDIR") kw = ::max(kw,mk_json_str(k).size()) ; @@ -370,8 +371,8 @@ R"({ append_to_string( script , "args=()\n" ) ; append_to_string( script , "type code | grep -q .vscode-server || args+=( \"--user-data-dir ${DEBUG_DIR}/vscode/user\" )\n" ) ; for( Dep const& dep : j->deps ) - if (dep.dflags[Dflag::Static]) append_to_string( script , "args+=( ",mk_shell_str("-g "+dep->name()),")\n" ) ; // list dependences file to open in vscode - append_to_string( script , "args+=(\"-g ${DEBUG_DIR}/cmd\")\n" ) ; + if (dep.dflags[Dflag::Static]) append_to_string( script , "args+=( ",mk_shell_str(dep->name()),")\n" ) ; // list dependences file to open in vscode + append_to_string( script , "args+=(\"${DEBUG_DIR}/cmd\")\n" ) ; append_to_string( script , "args+=(\"${DEBUG_DIR}/vscode/ldebug.code-workspace\")\n" ) ; append_to_string( script , "code -n -w ${args[@]} &" ) ; } else { diff --git a/src/lmakeserver/job.cc b/src/lmakeserver/job.cc index da4cf235..813a46b2 100644 --- a/src/lmakeserver/job.cc +++ b/src/lmakeserver/job.cc @@ -838,17 +838,17 @@ namespace Engine { state.reason |= {JobReasonTag::DepMissingStatic,+dep} ; dep_err = RunStatus::MissingStatic ; } - continue ; // dont check modifs and errors + continue ; // dont check modifs and errors } if (!care) goto Continue ; - dep_modif = !dep.up_to_date() ; - if ( dep_modif && status==Status::Ok && dep.no_trigger() ) { // no_trigger only applies to successful jobs + dep_modif = !dep.up_to_date( is_static && modif ) ; // after a modified dep, consider static deps as being fully accessed to avoid reruns due to previous errors + if ( dep_modif && status==Status::Ok && dep.no_trigger() ) { // no_trigger only applies to successful jobs trace("no_trigger",dep) ; - req->no_triggers.emplace(dep,req->no_triggers.size()) ; // record to repeat in summary, value is just to order summary in discovery order + req->no_triggers.emplace(dep,req->no_triggers.size()) ; // record to repeat in summary, value is just to order summary in discovery order dep_modif = false ; } - if ( +state.stamped_err ) goto Continue ; // we are already in error, no need to analyze errors any further - if ( !is_static && modif ) goto Continue ; // if not static, errors may be washed by previous modifs, dont record them + if ( +state.stamped_err ) goto Continue ; // we are already in error, no need to analyze errors any further + if ( !is_static && modif ) goto Continue ; // if not static, errors may be washed by previous modifs, dont record them if ( dep_modif ) state.reason |= {JobReasonTag::DepOutOfDate,+dep} ; // switch (dnd.ok(*cdri,dep.accesses)) { diff --git a/src/lmakeserver/node.x.hh b/src/lmakeserver/node.x.hh index 7cfa44da..bacf7e64 100644 --- a/src/lmakeserver/node.x.hh +++ b/src/lmakeserver/node.x.hh @@ -157,7 +157,7 @@ namespace Engine { ::string accesses_str() const ; ::string dflags_str () const ; // services - bool up_to_date () const ; + bool up_to_date (bool full=false) const ; void acquire_crc() ; } ; static_assert(sizeof(Dep)==16) ; @@ -358,7 +358,7 @@ namespace Engine { void full_refresh( bool report_no_file , ::vector const& reqs , ::string const& name ) { set_buildable() ; if (is_src_anti()) refresh_src_anti(report_no_file,reqs,name) ; - else manual_refresh (Req() ) ; // no manual_steady diagnostic as this may be because of another job + else manual_refresh (Req() ) ; // no manual_steady diagnostic as this may be because of another job } // RuleIdx conform_idx( ) const { if (_conform_idx<=MaxRuleIdx) return _conform_idx ; else return NoIdx ; } @@ -374,7 +374,7 @@ namespace Engine { Job cj = conform_job() ; return +cj && ( cj->is_special() || has_actual_job(cj) ) ; } - Bool3 ok(bool force_err=false) const { // if Maybe <=> not built + Bool3 ok(bool force_err=false) const { // if Maybe <=> not built switch (status()) { case NodeStatus::Plain : return No | !( force_err || conform_job()->err() ) ; case NodeStatus::Multi : return No ; @@ -411,14 +411,16 @@ namespace Engine { } // // services - bool read(Accesses a) const { // return true <= file was perceived different from non-existent, assuming access provided in a - if (crc==Crc::None ) return false ; // file does not exist, cannot perceive difference - if (a[Access::Stat]) return true ; // if file exists, stat is different + bool read(Accesses a) const { // return true <= file was perceived different from non-existent, assuming access provided in a + if (crc==Crc::None ) return false ; // file does not exist, cannot perceive difference + if (a[Access::Stat]) return true ; // if file exists, stat is different if (crc.is_lnk() ) return a[Access::Lnk] ; if (crc.is_reg() ) return a[Access::Reg] ; - else return +a ; // dont know if file is a link, any access may have perceived a difference + else return +a ; // dont know if file is a link, any access may have perceived a difference + } + bool up_to_date( DepDigest const& dd , bool full=false ) const { // only manage crc, not dates + return crc.match( dd.crc() , full?~Accesses():dd.accesses ) ; } - bool up_to_date(DepDigest const& dd) const { return crc.match(dd.crc(),dd.accesses) ; } // only manage crc, not dates // Manual manual_wash( ReqInfo& ri , bool lazy=false ) ; // @@ -430,13 +432,13 @@ namespace Engine { ::c_vector_view prio_job_tgts (RuleIdx prio_idx) const ; ::c_vector_view conform_job_tgts (ReqInfo const& ) const ; ::c_vector_view conform_job_tgts ( ) const ; - ::c_vector_view candidate_job_tgts( ) const ; // all jobs above prio provided in conform_idx + ::c_vector_view candidate_job_tgts( ) const ; // all jobs above prio provided in conform_idx // - void set_buildable( Req={} , DepDepth lvl=0 ) ; // data independent, may be pessimistic (Maybe instead of Yes), req is for error reporing only + void set_buildable( Req={} , DepDepth lvl=0 ) ; // data independent, may be pessimistic (Maybe instead of Yes), req is for error reporing only void set_pressure ( ReqInfo& , CoarseDelay pressure ) const ; // void propag_speculate( Req req , Bool3 speculate ) const { - /**/ if (speculate==Yes ) return ; // fast path : nothing to propagate + /**/ if (speculate==Yes ) return ; // fast path : nothing to propagate ReqInfo& ri = req_info(req) ; if (speculate>=ri.speculate) return ; ri.speculate = speculate ; _propag_speculate(ri) ; @@ -454,7 +456,7 @@ namespace Engine { bool/*modified*/ refresh( Crc , SigDate const& ={} ) ; void refresh( ) ; private : - void _set_buildable_raw( Req , DepDepth ) ; // req is for error reporting only + void _set_buildable_raw( Req , DepDepth ) ; // req is for error reporting only bool/*done*/ _make_pre ( ReqInfo& ) ; void _make_raw ( ReqInfo& , MakeAction , Bool3 speculate=Yes ) ; void _set_pressure_raw ( ReqInfo& ) const ; @@ -463,7 +465,7 @@ namespace Engine { Buildable _gather_special_rule_tgts( ::string const& name ) ; Buildable _gather_prio_job_tgts ( ::string const& name , Req , DepDepth lvl=0 ) ; Buildable _gather_prio_job_tgts ( Req r , DepDepth lvl=0 ) { - if (!rule_tgts()) return Buildable::No ; // fast path : avoid computing name() + if (!rule_tgts()) return Buildable::No ; // fast path : avoid computing name() else return _gather_prio_job_tgts( name() , r , lvl ) ; } // @@ -612,8 +614,8 @@ namespace Engine { // Dep // - inline bool Dep::up_to_date() const { - return is_crc && crc().match((*this)->crc,accesses) ; + inline bool Dep::up_to_date(bool full) const { + return is_crc && crc().match( (*this)->crc , full?~Accesses():accesses ) ; } inline void Dep::acquire_crc() { diff --git a/unit_tests/misc13.py b/unit_tests/misc13.py new file mode 100644 index 00000000..4d7156ab --- /dev/null +++ b/unit_tests/misc13.py @@ -0,0 +1,42 @@ +# This file is part of the open-lmake distribution (git@github.com:cesar-douady/open-lmake.git) +# Copyright (c) 2023 Doliam +# This program is free software: you can redistribute/modify under the terms of the GPL-v3 (https://www.gnu.org/licenses/gpl-3.0.html). +# This program is distributed WITHOUT ANY WARRANTY, without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + +if __name__!='__main__' : + + import sys + import time + + import lmake + from lmake.rules import Rule + + lmake.manifest = ( + 'Lmakefile.py' + , 'step.py' + ) + + from step import step + + class Hdep(Rule) : + target = r'hdep' + cmd = 'echo 2' + + class Sdep(Rule) : + target = r'sdep' + cmd = 'echo {step}' + + class Dut(Rule) : + target = 'dut' + dep = 'sdep' + cmd = '[ $(cat hdep) = {step} ] && cat sdep' + +else : + + import ut + + print('step=1',file=open('step.py','w')) + ut.lmake( 'dut' , may_rerun=1 , done=2 , failed=1 , rc=1 ) + + print('step=2',file=open('step.py','w')) + ut.lmake( 'dut' , done=2 ) # check sdep is not forgetten due to execution w/o hdep From 3e397f374705c04bdaf8f62867f663390cdd97de Mon Sep 17 00:00:00 2001 From: Cesar Douady Date: Tue, 30 Apr 2024 18:56:23 +0200 Subject: [PATCH 2/9] improve static deps management --- _lib/lmake_dbg.py | 2 -- src/lmakeserver/cmd.cc | 2 +- src/lmakeserver/job.cc | 11 ++++++----- unit_tests/misc13.py | 13 ++++++++----- unit_tests/misc8.py | 2 +- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/_lib/lmake_dbg.py b/_lib/lmake_dbg.py index a8dea3d1..b3af11b8 100644 --- a/_lib/lmake_dbg.py +++ b/_lib/lmake_dbg.py @@ -85,8 +85,6 @@ def run_pdb(dbg_dir,redirected,func,*args,**kwds) : def run_vscode(dbg_dir,redirected,func,*args,**kwds) : import json import os - import signal - signal.signal(signal.SIGPIPE,signal.SIG_IGN) # vscode seems to be sensitive to SIGPIPE try : # Write python process information to vscode debug workspace to allow gdb to attache to it workspace = dbg_dir + '/vscode/ldebug.code-workspace' diff --git a/src/lmakeserver/cmd.cc b/src/lmakeserver/cmd.cc index ec4164ee..b87a1745 100644 --- a/src/lmakeserver/cmd.cc +++ b/src/lmakeserver/cmd.cc @@ -374,7 +374,7 @@ R"({ if (dep.dflags[Dflag::Static]) append_to_string( script , "args+=( ",mk_shell_str(dep->name()),")\n" ) ; // list dependences file to open in vscode append_to_string( script , "args+=(\"${DEBUG_DIR}/cmd\")\n" ) ; append_to_string( script , "args+=(\"${DEBUG_DIR}/vscode/ldebug.code-workspace\")\n" ) ; - append_to_string( script , "code -n -w ${args[@]} &" ) ; + append_to_string( script , "code -n -w --password-store=basic ${args[@]} &" ) ; } else { ::vmap_ss env = _mk_env(start.env,report_end.end.dynamic_env) ; /**/ append_to_string( script , "exec env -i" , " \\\n" ) ; diff --git a/src/lmakeserver/job.cc b/src/lmakeserver/job.cc index 813a46b2..abfabe53 100644 --- a/src/lmakeserver/job.cc +++ b/src/lmakeserver/job.cc @@ -796,13 +796,14 @@ namespace Engine { bool sense_err = !dep.dflags[Dflag::IgnoreError] ; bool required = dep.dflags[Dflag::Required ] || is_static ; bool modif = state.stamped_modif || ri.force ; + bool may_care = care || (modif&&is_static) ; // if previous modif, consider static deps as fully accessed, as initially NodeReqInfo const* cdri = &dep->c_req_info(req) ; // avoid allocating req_info as long as not necessary NodeReqInfo * dri = nullptr ; // . NodeGoal dep_goal = - ri.full && care && (need_run(state)||archive) ? NodeGoal::Dsk - : ri.full && (care||sense_err) ? NodeGoal::Status - : required ? NodeGoal::Makable - : NodeGoal::None + ri.full && may_care && (need_run(state)||archive) ? NodeGoal::Dsk + : ri.full && (may_care||sense_err) ? NodeGoal::Status + : required ? NodeGoal::Makable + : NodeGoal::None ; if (!dep_goal) continue ; // this is not a dep (not static while asked for makable only) RestartDep : @@ -831,7 +832,7 @@ namespace Engine { critical_waiting |= is_critical ; } else { SWEAR(dnd.done(*cdri,dep_goal)) ; // after having called make, dep must be either waiting or done - bool dep_missing_dsk = ri.full && care && !dnd.done(*cdri,NodeGoal::Dsk) ; + bool dep_missing_dsk = ri.full && may_care && !dnd.done(*cdri,NodeGoal::Dsk) ; state.missing_dsk |= dep_missing_dsk ; // job needs this dep if it must run if (dep_goal==NodeGoal::Makable) { if ( is_static && required && dnd.ok(*cdri,dep.accesses)==Maybe ) { diff --git a/unit_tests/misc13.py b/unit_tests/misc13.py index 4d7156ab..972be8f4 100644 --- a/unit_tests/misc13.py +++ b/unit_tests/misc13.py @@ -20,23 +20,26 @@ class Hdep(Rule) : target = r'hdep' - cmd = 'echo 2' + cmd = 'echo {step}' class Sdep(Rule) : target = r'sdep' - cmd = 'echo {step}' + cmd = 'echo sdep' class Dut(Rule) : target = 'dut' - dep = 'sdep' - cmd = '[ $(cat hdep) = {step} ] && cat sdep' + deps = { 'SDEP' : 'sdep' } + cmd = '[ $(cat hdep) = 2 ] && cat sdep' else : + import os + import ut print('step=1',file=open('step.py','w')) ut.lmake( 'dut' , may_rerun=1 , done=2 , failed=1 , rc=1 ) print('step=2',file=open('step.py','w')) - ut.lmake( 'dut' , done=2 ) # check sdep is not forgetten due to execution w/o hdep + os.unlink('sdep') + ut.lmake( 'dut' , done=2 , steady=1 ) # check sdep is not forgetten due to execution w/o hdep diff --git a/unit_tests/misc8.py b/unit_tests/misc8.py index c0cf8774..576b2441 100644 --- a/unit_tests/misc8.py +++ b/unit_tests/misc8.py @@ -50,4 +50,4 @@ class Chk(Rule) : ut.lmake( 'dep' , done=1 ) print('step=2',file=open('step.py','w')) - ut.lmake( 'chk' , may_rerun=2 , done=5 ) # check that dut1 is remade after first run of dut2 despite having been done before + ut.lmake( 'chk' , may_rerun=1 , done=5 ) # check that dut1 is remade after first run of dut2 despite having been done before From 703f78a00df657ac28fe05f5f12537974e506be3 Mon Sep 17 00:00:00 2001 From: Cesar Douady Date: Wed, 1 May 2024 15:12:16 +0200 Subject: [PATCH 3/9] handle the O_EXCL flag of open() --- Manifest | 1 + src/autodep/record.cc | 46 +++++++++++++++++++++---------------------- src/autodep/record.hh | 4 ++-- unit_tests/misc14.py | 36 +++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 26 deletions(-) create mode 100644 unit_tests/misc14.py diff --git a/Manifest b/Manifest index 1ec92719..61cce6c8 100644 --- a/Manifest +++ b/Manifest @@ -193,6 +193,7 @@ unit_tests/misc1.py unit_tests/misc10.py unit_tests/misc11.py unit_tests/misc13.py +unit_tests/misc14.py unit_tests/misc2.py unit_tests/misc3.py unit_tests/misc4.py diff --git a/src/autodep/record.cc b/src/autodep/record.cc index a056af66..6e08e219 100644 --- a/src/autodep/record.cc +++ b/src/autodep/record.cc @@ -20,7 +20,7 @@ using namespace Time ; // // if strict, user might look at the st_size field which gives access to regular and link content -static constexpr Accesses UserStatAccess = StrictUserAccesses ? ~Accesses() : Accesses(Access::Stat) ; +static constexpr Accesses UserStatAccesses = StrictUserAccesses ? ~Accesses() : Accesses(Access::Stat) ; bool Record::s_static_report = false ; ::vmap_s * Record::s_deps = nullptr ; @@ -177,30 +177,28 @@ Record::Mkdir::Mkdir( Record& r , Path&& path , ::string&& c ) : Solve{r,::move( // However : // - if it is an official target, it is not a dep, whether you declare reading it or not // - else, we do not compute a CRC on it and its actual content is not guaranteed. What is important in this case is that the execution of the job does not see the content. -static bool _do_stat (int flags) { return flags&O_PATH ; } -static bool _do_read (int flags) { return !_do_stat(flags) && (flags&O_ACCMODE)!=O_WRONLY && !(flags&O_TRUNC) ; } -static bool _do_write(int flags) { return !_do_stat(flags) && (flags&O_ACCMODE)!=O_RDONLY ; } -Record::Open::Open( Record& r , Path&& path , int flags , ::string&& c ) : - Solve{ r , ::move(path) , bool(flags&O_NOFOLLOW) , _do_read(flags) , true/*allow_tmp_map*/ , to_string(c,::hex,'.',flags) } -, do_write{_do_write(flags)} -{ - bool do_read = _do_read(flags) ; - bool do_stat = _do_stat(flags) ; +// +static bool _no_follow(int flags) { return (flags&O_NOFOLLOW) || ( (flags&O_CREAT) && (flags&O_EXCL) ) ; } +static bool _do_stat (int flags) { return flags&O_PATH || ( (flags&O_CREAT) && (flags&O_EXCL) ) ; } +static bool _do_read (int flags) { return !(flags&O_PATH) && (flags&O_ACCMODE)!=O_WRONLY && !(flags&O_TRUNC) ; } +static bool _do_write (int flags) { return !(flags&O_PATH) && (flags&O_ACCMODE)!=O_RDONLY ; } +// +Record::Open::Open( Record& r , Path&& path , int flags , ::string&& c ) : Solve{ r , ::move(path) , _no_follow(flags) , _do_read(flags) , true/*allow_tmp_map*/ , to_string(c,::hex,'.',flags) } { + if ( flags&(O_DIRECTORY|O_TMPFILE) ) return ; // we already solved, this is enough + if ( !(flags&O_PATH) && s_autodep_env().ignore_stat ) return ; + if ( file_loc>FileLoc::Dep ) return ; // - if ( flags&(O_DIRECTORY|O_TMPFILE) ) { file_loc = FileLoc::Ext ; return ; } // we already solved, this is enough - if ( do_stat && s_autodep_env().ignore_stat ) { file_loc = FileLoc::Ext ; return ; } - if ( file_loc>FileLoc::Dep ) return ; + bool do_stat = _do_stat (flags) ; + bool do_read = _do_read (flags) ; + bool do_write = _do_write(flags) && file_loc==FileLoc::Repo ; // - if (!do_write) { - if (do_read) r._report_dep ( ::move(real) , accesses|Access::Reg|UserStatAccess , c+".rd" ) ; // user might do fstat on returned fd and if strict user might look at st_size field - else if (do_stat) r._report_dep ( ::move(real) , accesses |UserStatAccess , c+".path" ) ; // . - } else if (file_loc==FileLoc::Repo) { - if (do_read) r._report_update( ::move(real) , accesses|Access::Reg|UserStatAccess , c+".upd" ) ; // file date may be updated if created, user might do a meaningful fstat - else r._report_update( ::move(real) , accesses , c+".wr" ) ; // file date may be updated if created, user cannot see file stat before the write - } else { - if (do_read) r._report_dep ( ::move(real) , accesses|Access::Reg|UserStatAccess , c+".upd" ) ; // in src dirs, only the read side is reported - else r._report_dep ( ::move(real) , accesses , c+".wr" ) ; // . - } + c += '.' ; + if (do_stat ) { c += 'S' ; accesses |= UserStatAccesses ; } + if (do_read ) { c += 'R' ; accesses |= UserStatAccesses|Access::Reg ; } + if (do_write) c += 'W' ; + // + if ( do_write ) { r._report_update( ::move(real) , accesses , ::move(c) ) ; confirm = true ; } + else if ( do_read || do_stat ) r._report_dep ( ::move(real) , accesses , ::move(c) ) ; } Record::Read::Read( Record& r , Path&& path , bool no_follow , bool keep_real , bool allow_tmp_map , ::string&& c ) : Solve{r,::move(path),no_follow,true/*read*/,allow_tmp_map,c} { @@ -303,7 +301,7 @@ Record::Rename::Rename( Record& r , Path&& src_ , Path&& dst_ , bool exchange , } Record::Stat::Stat( Record& r , Path&& path , bool no_follow , ::string&& c ) : Solve{r,::move(path),no_follow,true/*read*/,true/*allow_tmp_map*/,c} { - if ( !s_autodep_env().ignore_stat && file_loc<=FileLoc::Dep ) r._report_dep( ::move(real) , accesses|UserStatAccess , ::move(c) ) ; + if ( !s_autodep_env().ignore_stat && file_loc<=FileLoc::Dep ) r._report_dep( ::move(real) , accesses|UserStatAccesses , ::move(c) ) ; } Record::Symlnk::Symlnk( Record& r , Path&& p , ::string&& c ) : Solve{r,::move(p),true/*no_follow*/,false/*read*/,true/*allow_tmp_map*/,c} { diff --git a/src/autodep/record.hh b/src/autodep/record.hh index 1bb9fff6..e028c206 100644 --- a/src/autodep/record.hh +++ b/src/autodep/record.hh @@ -279,9 +279,9 @@ public : Open() = default ; Open( Record& , Path&& , int flags , ::string&& comment ) ; // services - int operator()( Record& r , int rc ) { { if (do_write) r._report_confirm(file_loc,rc>=0) ; } return rc ; } + int operator()( Record& r , int rc ) { { if (confirm) r._report_confirm(file_loc,rc>=0) ; } return rc ; } // data - bool do_write = false ; + bool confirm = false ; } ; struct Read : Solve { Read() = default ; diff --git a/unit_tests/misc14.py b/unit_tests/misc14.py new file mode 100644 index 00000000..f261e09e --- /dev/null +++ b/unit_tests/misc14.py @@ -0,0 +1,36 @@ +# This file is part of the open-lmake distribution (git@github.com:cesar-douady/open-lmake.git) +# Copyright (c) 2023 Doliam +# This program is free software: you can redistribute/modify under the terms of the GPL-v3 (https://www.gnu.org/licenses/gpl-3.0.html). +# This program is distributed WITHOUT ANY WARRANTY, without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + +if __name__!='__main__' : + + import sys + import time + + import lmake + from lmake.rules import Rule + + lmake.manifest = ('Lmakefile.py',) + + class DutSh(Rule) : + targets = { 'DUT' : r'dut_sh{N*:\d}' } + cmd = ''' + [ -f dut_sh1 ] && exit 1 ; echo 1 >dut_sh1 + [ -f dut_sh2 ] && exit 1 ; echo 2 >dut_sh2 + ''' + + class DutPy(Rule) : + targets = { 'DUT' : r'dut_py{N*:\d}' } + def cmd() : + print(1,file=open('dut_py1','x')) + print(2,file=open('dut_py2','x')) + +else : + + import ut + + print('manual',file=open('dut_sh2','w')) + print('manual',file=open('dut_py2','w')) + ut.lmake( 'dut_sh1' , rerun=1 , done=1 ) # check dut_sh2 is quarantined and job rerun + ut.lmake( 'dut_py1' , rerun=1 , done=1 , new=1 ) # check dut_py2 is quarantined and job rerun From 8cdb1ad49d779505006ddd6d032a2e6cee2855f0 Mon Sep 17 00:00:00 2001 From: Cesar Douady Date: Thu, 2 May 2024 00:19:25 +0200 Subject: [PATCH 4/9] fix rare case of crash after check_deps or depend verbose --- src/lmakeserver/backend.cc | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/lmakeserver/backend.cc b/src/lmakeserver/backend.cc index 247dfab1..ca73748a 100644 --- a/src/lmakeserver/backend.cc +++ b/src/lmakeserver/backend.cc @@ -17,11 +17,18 @@ namespace Backends { void send_reply( JobIdx job , JobMngtRpcReply&& jmrr ) { Lock lock { Backend::_s_mutex } ; auto it = Backend::_s_start_tab.find(job) ; - if (it==Backend::_s_start_tab.end()) return ; // job is dead without waiting for reply, curious but possible - Backend::StartEntry::Conn const& conn = it->second.conn ; - ClientSockFd fd ( conn.host , conn.port ) ; - jmrr.seq_id = conn.seq_id ; - OMsgBuf().send( fd , jmrr ) ; // XXX : straighten out Fd : Fd must not detach on mv and Epoll must take an AutoCloseFd as arg to take close resp. + if (it==Backend::_s_start_tab.end()) return ; // job is dead without waiting for reply, curious but possible + Backend::StartEntry const& e = it->second ; + try { + jmrr.seq_id = e.conn.seq_id ; + ClientSockFd fd( e.conn.host , e.conn.port , 3/*n_trials*/ ) ; + OMsgBuf().send( fd , jmrr ) ; // XXX : straighten out Fd : Fd must not detach on mv and Epoll must take an AutoCloseFd as arg to take close resp. + } catch (...) { // if we cannot connect to job, assume it is dead while we processed the request + Backend::_s_deferred_wakeup_thread->emplace_after( + g_config.network_delay + , Backend::DeferredEntry { e.conn.seq_id , JobExec(Job(job),e.conn.host,e.start_date,New) } + ) ; + } } // @@ -424,7 +431,7 @@ namespace Backends { DF} Job job { jmrr.job } ; Trace trace(BeChnl,"_s_handle_job_mngt",jmrr) ; - { Lock lock { _s_mutex } ; // prevent sub-backend from manipulating _s_start_tab from main thread, lock for minimal time + { Lock lock { _s_mutex } ; // prevent sub-backend from manipulating _s_start_tab from main thread, lock for minimal time // keep_fd auto it = _s_start_tab.find(+job) ; if (it==_s_start_tab.end() ) { trace("not_in_tab" ) ; return false ; } StartEntry& entry = it->second ; if (entry.conn.seq_id!=jmrr.seq_id) { trace("bad_seq_id",entry.conn.seq_id,jmrr.seq_id) ; return false ; } @@ -642,7 +649,7 @@ namespace Backends { be->addr = ServerSockFd::s_addr(ifce) ; } try { be->config(cfg.dct,dynamic) ; be->config_err.clear() ; trace("ready",t ) ; } - catch (::string const& e) { SWEAR(+e) ; be->config_err = e ; trace("err" ,t,e) ; } // empty config_err means ready + catch (::string const& e) { SWEAR(+e) ; be->config_err = e ; trace("err" ,t,e) ; } // empty config_err means ready } job_start_thread.wait_started() ; job_mngt_thread .wait_started() ; From 48cd4a355c4cdf97b00a5a5104317fbd88f3d841 Mon Sep 17 00:00:00 2001 From: Cesar Douady Date: Thu, 2 May 2024 09:36:33 +0200 Subject: [PATCH 5/9] fix a rerun loop in some cases of manual targets --- src/rpc_job.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/rpc_job.cc b/src/rpc_job.cc index 58e5bd50..e3455d0f 100644 --- a/src/rpc_job.cc +++ b/src/rpc_job.cc @@ -40,10 +40,8 @@ ::pair_s do_file_actions( ::vector_s* unlnks/*out*/ , ::vmap_s do_file_actions( ::vector_s* unlnks/*out*/ , ::vmap_spush_back(f) ; ok &= done ; } break ; - case FileActionTag::Rmdir : + case FileActionTag::Uniquify : if (uniquify(nfs_guard.change(f))) append_to_string(msg,"uniquified ",mk_file(f),'\n') ; break ; + case FileActionTag::Mkdir : mkdir(f,nfs_guard) ; break ; + case FileActionTag::Rmdir : if (!keep_dirs.contains(f)) try { rmdir(nfs_guard.change(f)) ; } catch (::string const&) { for( ::string d=f ; +d ; d = dir_name(d) ) if (!keep_dirs.insert(d).second) break ; } // if a dir cannot rmdir'ed, no need to try those uphill From 1a94c9c6f1a8b4091fa2cc625943947792baa9b9 Mon Sep 17 00:00:00 2001 From: Cesar Douady Date: Thu, 2 May 2024 19:55:04 +0200 Subject: [PATCH 6/9] fix crash with depend verbose + rare crashes in presence of lot of errors --- src/autodep/record.hh | 2 +- src/lmakeserver/codec.cc | 6 ++++-- src/lmakeserver/job.cc | 4 ++++ src/lmakeserver/job.x.hh | 2 +- src/lmakeserver/main.cc | 10 ++++++---- src/lmakeserver/node.x.hh | 2 +- src/rpc_job.cc | 2 +- 7 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/autodep/record.hh b/src/autodep/record.hh index e028c206..627b0218 100644 --- a/src/autodep/record.hh +++ b/src/autodep/record.hh @@ -97,7 +97,7 @@ private : else return IMsgBuf().receive(report_fd()) ; } // - void _report_access( JobExecRpcReq&& jerr ) const ; + void _report_access( JobExecRpcReq&& jerr ) const ; void _report_access( ::string&& f , FileInfo fi , Accesses a , bool write , ::string&& c={} ) const { _report_access({ Proc::Access , {{::move(f),fi}} , {.write=Maybe&write,.accesses=a} , ::move(c) }) ; } diff --git a/src/lmakeserver/codec.cc b/src/lmakeserver/codec.cc index 22144ba5..da308249 100644 --- a/src/lmakeserver/codec.cc +++ b/src/lmakeserver/codec.cc @@ -281,8 +281,10 @@ namespace Codec { case JobMngtProc::Decode : jmrr = cc.decode() ; break ; case JobMngtProc::Encode : jmrr = cc.encode() ; break ; DF} - jmrr.fd = cc.fd ; // seq_id will be filled in by send_reply - Backends::send_reply( cc.job , ::move(jmrr) ) ; + if (cc.fd) { + jmrr.fd = cc.fd ; // seq_id will be filled in by send_reply + Backends::send_reply( cc.job , ::move(jmrr) ) ; + } } } diff --git a/src/lmakeserver/job.cc b/src/lmakeserver/job.cc index abfabe53..43428b09 100644 --- a/src/lmakeserver/job.cc +++ b/src/lmakeserver/job.cc @@ -911,6 +911,10 @@ namespace Engine { } Run : trace("run",pre_reason,ri,run_status) ; + if ( ri.state.missing_dsk) { + ri.reset() ; + goto RestartAnalysis ; + } SWEAR(!ri.state.missing_dsk) ; // cant run if we are missing some deps on disk if (rule->n_submits) { if (ri.n_submits>=rule->n_submits) { diff --git a/src/lmakeserver/job.x.hh b/src/lmakeserver/job.x.hh index dee94dd4..b8d00421 100644 --- a/src/lmakeserver/job.x.hh +++ b/src/lmakeserver/job.x.hh @@ -232,7 +232,7 @@ namespace Engine { private : Step _step:3 = {} ; // 3 bits } ; - static_assert(sizeof(JobReqInfo)==48) ; // check expected size XXX : optimize size, can be 32 + static_assert(sizeof(JobReqInfo)==48) ; // check expected size, XXX : optimize size, can be 32 } diff --git a/src/lmakeserver/main.cc b/src/lmakeserver/main.cc index c030e02b..e82607d5 100644 --- a/src/lmakeserver/main.cc +++ b/src/lmakeserver/main.cc @@ -376,10 +376,12 @@ bool/*interrupted*/ engine_loop() { ::vector deps ; deps.reserve(ecjm.deps.size()) ; for( auto const& [dn,dd] : ecjm.deps ) deps.emplace_back(Node(dn),dd) ; JobMngtRpcReply jmrr = je.job_info(ecjm.proc,deps) ; - jmrr.fd = ecjm.fd ; // seq_id will be filled in by send_reply - //vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv - Backends::send_reply( +je , ::move(jmrr) ) ; - //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + if (+ecjm.fd) { + jmrr.fd = ecjm.fd ; // seq_id will be filled in by send_reply + //vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv + Backends::send_reply( +je , ::move(jmrr) ) ; + //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + } } break ; DF} } break ; diff --git a/src/lmakeserver/node.x.hh b/src/lmakeserver/node.x.hh index bacf7e64..6a867ff4 100644 --- a/src/lmakeserver/node.x.hh +++ b/src/lmakeserver/node.x.hh @@ -212,7 +212,7 @@ namespace Engine { DepsIter& operator++(int) { return ++*this ; } DepsIter& operator++( ) { if (i_chunkhdr.sz) i_chunk++ ; // go to next item in chunk - else { i_chunk = 0 ; hdr = hdr->next() ; } // go to next chunk } + else { i_chunk = 0 ; hdr = hdr->next() ; } // go to next chunk return *this ; } // data diff --git a/src/rpc_job.cc b/src/rpc_job.cc index e3455d0f..f5538e9a 100644 --- a/src/rpc_job.cc +++ b/src/rpc_job.cc @@ -199,7 +199,7 @@ ::ostream& operator<<( ::ostream& os , JobMngtRpcReq const& jmrr ) { return os <<')' ; } -JobMngtRpcReq::JobMngtRpcReq( SI si , JI j , Fd fd_ , JobExecRpcReq&& jerr ) : seq_id{si} , job{j} , fd{fd_} { +JobMngtRpcReq::JobMngtRpcReq( SI si , JI j , Fd fd_ , JobExecRpcReq&& jerr ) : seq_id{si} , job{j} , fd{jerr.sync?fd_:Fd()} { switch (jerr.proc) { case JobExecProc::Decode : proc = P::Decode ; ctx = ::move(jerr.ctx) ; file = ::move(jerr.files[0].first) ; txt = ::move(jerr.txt) ; break ; case JobExecProc::Encode : proc = P::Encode ; ctx = ::move(jerr.ctx) ; file = ::move(jerr.files[0].first) ; txt = ::move(jerr.txt) ; min_len = jerr.min_len ; break ; From 584a0e07dc46120fd0a25c52f834281113ace8a6 Mon Sep 17 00:00:00 2001 From: Cesar Douady Date: Fri, 3 May 2024 10:11:55 +0200 Subject: [PATCH 7/9] fixup --- src/autodep/gather.cc | 6 ++++-- src/lmakeserver/codec.cc | 6 ++---- src/lmakeserver/main.cc | 10 ++++------ src/rpc_job.cc | 3 ++- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/autodep/gather.cc b/src/autodep/gather.cc index 3e9de9ab..de24b060 100644 --- a/src/autodep/gather.cc +++ b/src/autodep/gather.cc @@ -202,10 +202,11 @@ void Gather::_send_to_server ( Fd fd , Jerr&& jerr ) { case Proc::ChkDeps : reorder(false/*at_end*/) ; break ; // ensure server sees a coherent view case Proc::DepVerbose : _new_accesses(fd,::copy(jerr)) ; break ; // - case Proc::Decode : SWEAR(jerr.files.size()==1) ; _codec_files[fd] = Codec::mk_decode_node( jerr.files[0].first , jerr.ctx , jerr.txt ) ; break ; - case Proc::Encode : SWEAR(jerr.files.size()==1) ; _codec_files[fd] = Codec::mk_encode_node( jerr.files[0].first , jerr.ctx , jerr.txt ) ; break ; + case Proc::Decode : SWEAR( jerr.sync && jerr.files.size()==1 ) ; _codec_files[fd] = Codec::mk_decode_node( jerr.files[0].first , jerr.ctx , jerr.txt ) ; break ; + case Proc::Encode : SWEAR( jerr.sync && jerr.files.size()==1 ) ; _codec_files[fd] = Codec::mk_encode_node( jerr.files[0].first , jerr.ctx , jerr.txt ) ; break ; default : ; } + if (!jerr.sync) fd = {} ; // dont reply if not sync try { JobMngtRpcReq jmrr ; if (jerr.proc==JobExecProc::ChkDeps) jmrr = { JobMngtProc::ChkDeps , seq_id , job , fd , cur_deps_cb() } ; @@ -451,6 +452,7 @@ Status Gather::exec_child( ::vector_s const& args , Fd cstdin , Fd cstdout , Fd case JobMngtProc::ChkDeps : if (jmrr.ok==Maybe) { set_status(Status::ChkDeps) ; kill_step = KillStep::Kill ; rfd = {} ; } break ; case JobMngtProc::Decode : case JobMngtProc::Encode : { + SWEAR(+jmrr.fd) ; auto it = _codec_files.find(jmrr.fd) ; _new_access( rfd , PD(New) , ::move(it->second) , {.accesses=Access::Reg} , jmrr.crc , false/*parallel*/ , ::string(snake(jmrr.proc)) ) ; _codec_files.erase(it) ; diff --git a/src/lmakeserver/codec.cc b/src/lmakeserver/codec.cc index da308249..22144ba5 100644 --- a/src/lmakeserver/codec.cc +++ b/src/lmakeserver/codec.cc @@ -281,10 +281,8 @@ namespace Codec { case JobMngtProc::Decode : jmrr = cc.decode() ; break ; case JobMngtProc::Encode : jmrr = cc.encode() ; break ; DF} - if (cc.fd) { - jmrr.fd = cc.fd ; // seq_id will be filled in by send_reply - Backends::send_reply( cc.job , ::move(jmrr) ) ; - } + jmrr.fd = cc.fd ; // seq_id will be filled in by send_reply + Backends::send_reply( cc.job , ::move(jmrr) ) ; } } diff --git a/src/lmakeserver/main.cc b/src/lmakeserver/main.cc index e82607d5..c030e02b 100644 --- a/src/lmakeserver/main.cc +++ b/src/lmakeserver/main.cc @@ -376,12 +376,10 @@ bool/*interrupted*/ engine_loop() { ::vector deps ; deps.reserve(ecjm.deps.size()) ; for( auto const& [dn,dd] : ecjm.deps ) deps.emplace_back(Node(dn),dd) ; JobMngtRpcReply jmrr = je.job_info(ecjm.proc,deps) ; - if (+ecjm.fd) { - jmrr.fd = ecjm.fd ; // seq_id will be filled in by send_reply - //vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv - Backends::send_reply( +je , ::move(jmrr) ) ; - //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - } + jmrr.fd = ecjm.fd ; // seq_id will be filled in by send_reply + //vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv + Backends::send_reply( +je , ::move(jmrr) ) ; + //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ } break ; DF} } break ; diff --git a/src/rpc_job.cc b/src/rpc_job.cc index f5538e9a..eed2ae8e 100644 --- a/src/rpc_job.cc +++ b/src/rpc_job.cc @@ -199,7 +199,8 @@ ::ostream& operator<<( ::ostream& os , JobMngtRpcReq const& jmrr ) { return os <<')' ; } -JobMngtRpcReq::JobMngtRpcReq( SI si , JI j , Fd fd_ , JobExecRpcReq&& jerr ) : seq_id{si} , job{j} , fd{jerr.sync?fd_:Fd()} { +JobMngtRpcReq::JobMngtRpcReq( SI si , JI j , Fd fd_ , JobExecRpcReq&& jerr ) : seq_id{si} , job{j} , fd{fd_} { + SWEAR( jerr.sync || !fd , jerr,fd ) ; switch (jerr.proc) { case JobExecProc::Decode : proc = P::Decode ; ctx = ::move(jerr.ctx) ; file = ::move(jerr.files[0].first) ; txt = ::move(jerr.txt) ; break ; case JobExecProc::Encode : proc = P::Encode ; ctx = ::move(jerr.ctx) ; file = ::move(jerr.files[0].first) ; txt = ::move(jerr.txt) ; min_len = jerr.min_len ; break ; From 6c951cb023f52226309f1d7cfdb5b86d35fe7fd7 Mon Sep 17 00:00:00 2001 From: Cesar Douady Date: Fri, 3 May 2024 14:55:55 +0200 Subject: [PATCH 8/9] fix rare crash in presence of lost jobs --- src/lmakeserver/backend.cc | 3 +++ src/lmakeserver/backend.x.hh | 8 ++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/lmakeserver/backend.cc b/src/lmakeserver/backend.cc index ca73748a..66ec5989 100644 --- a/src/lmakeserver/backend.cc +++ b/src/lmakeserver/backend.cc @@ -19,6 +19,7 @@ namespace Backends { auto it = Backend::_s_start_tab.find(job) ; if (it==Backend::_s_start_tab.end()) return ; // job is dead without waiting for reply, curious but possible Backend::StartEntry const& e = it->second ; + if (!e ) return ; // . try { jmrr.seq_id = e.conn.seq_id ; ClientSockFd fd( e.conn.host , e.conn.port , 3/*n_trials*/ ) ; @@ -166,6 +167,7 @@ namespace Backends { void Backend::_s_wakeup_remote( JobIdx job , StartEntry::Conn const& conn , SigDate const& start_date , JobMngtProc proc ) { Trace trace(BeChnl,"_s_wakeup_remote",job,conn,proc) ; + SWEAR(conn.seq_id,job,conn) ; try { ClientSockFd fd(conn.host,conn.port) ; OMsgBuf().send( fd , JobMngtRpcReply(proc,conn.seq_id) ) ; // XXX : straighten out Fd : Fd must not detach on mv and Epoll must take an AutoCloseFd as arg to take close resp. @@ -510,6 +512,7 @@ namespace Backends { for( auto jit = _s_start_tab.begin() ; jit!=_s_start_tab.end() ;) { // /!\ we erase entries while iterating JobIdx j = jit->first ; StartEntry& e = jit->second ; + if (!e) { jit++ ; continue ; } if (ri) { if ( e.reqs.size()==1 && e.reqs[0]==ri ) goto Kill ; for( auto it = e.reqs.begin() ; it!=e.reqs.end() ; it++ ) { // e.reqs is a non-sorted vector, we must search ri by hand diff --git a/src/lmakeserver/backend.x.hh b/src/lmakeserver/backend.x.hh index ff41b5f5..64ff1191 100644 --- a/src/lmakeserver/backend.x.hh +++ b/src/lmakeserver/backend.x.hh @@ -54,6 +54,10 @@ namespace Backends { friend ::ostream& operator<<( ::ostream& , StartEntry const& ) ; struct Conn { friend ::ostream& operator<<( ::ostream& , Conn const& ) ; + // accesses + bool operator+() const { return seq_id ; } + bool operator!() const { return !+*this ; } + // data in_addr_t host = NoSockAddr ; in_port_t port = 0 ; SeqId seq_id = 0 ; @@ -63,8 +67,8 @@ namespace Backends { StartEntry( ) = default ; StartEntry(NewType) { open() ; } // accesses - bool operator+() const { return conn.seq_id ; } - bool operator!() const { return !+*this ; } + bool operator+() const { return +conn ; } + bool operator!() const { return !+*this ; } bool useful () const ; // services void open() { From 1426ecbd2d850862a1b83281d8ad14a2b3a122dd Mon Sep 17 00:00:00 2001 From: Cesar Douady Date: Sat, 4 May 2024 18:06:32 +0200 Subject: [PATCH 9/9] fix crashes when playing with ^C --- src/lmakeserver/main.cc | 7 +++++-- src/lmakeserver/req.cc | 2 -- src/lmakeserver/req.x.hh | 1 + 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/lmakeserver/main.cc b/src/lmakeserver/main.cc index c030e02b..f513e45f 100644 --- a/src/lmakeserver/main.cc +++ b/src/lmakeserver/main.cc @@ -146,7 +146,6 @@ void reqs_thread_func( ::stop_token stop , Fd in_fd , Fd out_fd ) { for(;;) { ::vector events = epoll.wait() ; bool new_fd = false ; - ::umap req_tab ; for( Epoll::Event event : events ) { EventKind kind = event.data() ; Fd fd = event.fd() ; @@ -157,7 +156,8 @@ void reqs_thread_func( ::stop_token stop , Fd in_fd , Fd out_fd ) { // - lmake foo // - touch Lmakefile.py // - lmake bar => maybe we get this request in the same poll as the end of lmake foo and we would eroneously say that it cannot be processed - // solution is to delay master events after other events and ignore them if we are done inbetween + // solution is to delay Master event after other events and ignore them if we are done inbetween + // note that there may at most a single Master event case EventKind::Master : SWEAR( !new_fd , new_fd ) ; new_fd = true ; @@ -190,6 +190,7 @@ void reqs_thread_func( ::stop_token stop , Fd in_fd , Fd out_fd ) { switch (rrr.proc) { case ReqProc::Make : { Req r{New} ; + r.zombie(false) ; in_tab.at(fd).second = r ; //vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv g_engine_queue.emplace( rrr.proc , r , fd , ofd , rrr.files , rrr.options ) ; @@ -329,6 +330,7 @@ bool/*interrupted*/ engine_loop() { //vvvvvvvvv req.close() ; //^^^^^^^^^ + if (it->second.killed ) req.dealloc() ; // dealloc when req can be reused if (it->second.in!=it->second.out) ::close (it->second.out ) ; else if (it->second.killed ) ::close (it->second.out ) ; // we are after Kill, finalize close of file descriptor else ::shutdown(it->second.out,SHUT_WR) ; // we are before Kill, shutdown until final close upon Close @@ -344,6 +346,7 @@ bool/*interrupted*/ engine_loop() { if ( +req && +*req && req_active ) req.kill() ; // ^^^^^^^^^^ if (req_active ) it->second.killed = true ; + else req.dealloc() ; // dealloc when req can be reused if (ecr.in_fd!=ecr.out_fd) ::close (ecr.in_fd ) ; else if (!req_active ) ::close (ecr.in_fd ) ; // we are after Close, finalize close of file descriptor else ::shutdown(ecr.in_fd,SHUT_RD) ; // we are before Close, shutdown until final close upon Close diff --git a/src/lmakeserver/req.cc b/src/lmakeserver/req.cc index 3ccbf6d0..212e55a1 100644 --- a/src/lmakeserver/req.cc +++ b/src/lmakeserver/req.cc @@ -110,8 +110,6 @@ namespace Engine { _s_reqs_by_eta.pop_back() ; } (*this)->clear() ; - zombie(false) ; - s_small_ids.release(+*this) ; } void Req::inc_rule_exec_time( Rule rule , Delay delta , Tokens1 tokens1 ) { diff --git a/src/lmakeserver/req.x.hh b/src/lmakeserver/req.x.hh index ebdc8911..8deca20f 100644 --- a/src/lmakeserver/req.x.hh +++ b/src/lmakeserver/req.x.hh @@ -80,6 +80,7 @@ namespace Engine { void kill ( ) ; void close ( ) ; void chk_end( ) ; + void dealloc( ) { s_small_ids.release(+*this) ; } // void inc_rule_exec_time( Rule , Delay delta , Tokens1 ) ; void new_exec_time ( JobData const& , bool remove_old , bool add_new , Delay old_exec_time ) ;