Skip to content

Commit f1bb32a

Browse files
adrien-cottexdelaruelle
authored andcommitted
Restore dependent reload list when an evaluation is rolled back
When the unload of a module triggers the unload phase of several dependent modules and one of them fails, the evaluation is rolled back but the dependent reload list keeps the modules queued before the failure. Their properties were never saved as the unload phase stopped before reaching them. If the sequence then continues, like a switch with switch_unload absent from abort_on_error, the reload phase looks up these missing properties and raises an internal Tcl error. Save the dependent reload list and the saved properties of reloading modules along with the other settings pushed before an evaluation, so a rollback restores them to their state prior the failed evaluation. Dependents restored in the loaded environment by this rollback are thus not reloaded anymore. Assisted-by: Claude:claude-fable-5-1 Signed-off-by: Adrien Cotte <adrien.cotte@asplus.fr> Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
1 parent 894efe8 commit f1bb32a

3 files changed

Lines changed: 35 additions & 2 deletions

File tree

‎NEWS.rst‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,11 @@ Modules 5.7.0 (not yet released)
267267
* Keep non-sticky requirements of sticky modules loaded during a
268268
:subcmd:`purge` and explicitly report why their unload is skipped according
269269
to the :mconfig:`sticky_purge` configuration. (fix issue #582)
270+
* Fix internal error raised by :subcmd:`switch` sub-command when the unload
271+
of the switched-off module fails after some of its dependent modules were
272+
unloaded for reload, and switch sequence continues as ``switch_unload`` is
273+
not set in :mconfig:`abort_on_error`. Dependent modules restored by the
274+
failed unload are not reloaded anymore.
270275

271276
.. _CVE-2026-85013: https://github.com/envmodules/modules/security/advisories/GHSA-8hrw-p88g-qhmg
272277

‎tcl/modeval.tcl‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,7 @@ proc pushSettings {} {
593593
g_moduleNPODepend g_dependNPOHash g_prereqViolation\
594594
g_prereqNPOViolation g_conflictViolation g_moduleUnmetDep\
595595
g_unmetDepHash g_moduleEval g_moduleHiddenEval g_scanModuleVariant\
596-
g_savedLoReqOfReloadMod g_savedLoReqOfUnloadMod\
596+
g_savedLoReqOfReloadMod g_savedLoReqOfUnloadMod g_savedPropsOfReloadMod\
597597
g_loadedModulePrereqPath g_tagHash g_loadedByModroot g_modrootByLoaded\
598598
g_loadedModulePosition g_modrootByLoadedConflict\
599599
g_loadedConflictByModroot} {
@@ -605,7 +605,7 @@ proc pushSettings {} {
605605

606606
# save non-array variable and indication if it was set
607607
foreach var {g_changeDir g_stdoutPuts g_prestdoutPuts g_return_text\
608-
g_uReqUnFromDepReList} {
608+
g_uReqUnFromDepReList g_depReList} {
609609
##nagelfar ignore #2 Badly formed if statement
610610
lappend save_var $var {*}[if {[info exists ::$var]} {list 1 [set\
611611
::$var]} {list 0 {}}]

‎testsuite/modules.50-cmds/616-abort_on_error-switch.exp‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,34 @@ append ts_depre_unload_error_force [msg_top_switch foo/1.0 setenv/1.0 {} {} {} {
878878
testouterr_cmd bash {switch --force foo/1.0 setenv/1.0} $ans_unload_fail_depre_force $ts_depre_unload_error_force
879879

880880

881+
# another dependent module was unloaded prior failing dependent unload
882+
setenv_loaded_module [list foo/1.0 lerr/1.0 err/1.0] [list $mp/foo/1.0 $mp/lerr/1.0 $mp/err/1.0]
883+
setenv_var __MODULES_LMPREREQ lerr/1.0&foo:err/1.0&err/1.0|foo
884+
885+
set ans_unload_fail_depre_other_continue [list]
886+
lappend ans_unload_fail_depre_other_continue [list set TS1 {}]
887+
lappend ans_unload_fail_depre_other_continue [list set TS2 {}]
888+
lappend ans_unload_fail_depre_other_continue [list set _LMFILES_ $mp/foo/1.0:$mp/lerr/1.0:$mp/err/1.0:$mp/setenv/1.0]
889+
lappend ans_unload_fail_depre_other_continue [list set LOADEDMODULES foo/1.0:lerr/1.0:err/1.0:setenv/1.0]
890+
lappend ans_unload_fail_depre_other_continue [list ERR]
891+
892+
testouterr_cmd bash {switch foo/1.0 setenv/1.0} $ans_unload_fail_depre_other_continue $ts_depre_unload_error
893+
894+
set ans_unload_fail_depre_other_force [list]
895+
lappend ans_unload_fail_depre_other_force [list set TS1 {}]
896+
lappend ans_unload_fail_depre_other_force [list set TS2 {}]
897+
lappend ans_unload_fail_depre_other_force [list set __MODULES_LMPREREQ err/1.0&err/1.0|foo]
898+
lappend ans_unload_fail_depre_other_force [list set _LMFILES_ $mp/setenv/1.0:$mp/err/1.0]
899+
lappend ans_unload_fail_depre_other_force [list set LOADEDMODULES setenv/1.0:err/1.0]
900+
901+
set ts_depre_unload_error_other_force [msg_unload err/1.0 [msg_modwarn msg {error msg} $mp/err/1.0 2 {} {} {} $custom_error_trace]]\n\n
902+
append ts_depre_unload_error_other_force [msg_top_switch foo/1.0 setenv/1.0 {} {} {} {} [list {err/1.0 lerr/1.0} err/1.0]]
903+
testouterr_cmd bash {switch --force foo/1.0 setenv/1.0} $ans_unload_fail_depre_other_force $ts_depre_unload_error_other_force
904+
905+
setenv_loaded_module [list foo/1.0 err/1.0] [list $mp/foo/1.0 $mp/err/1.0]
906+
setenv_var __MODULES_LMPREREQ err/1.0&err/1.0|foo
907+
908+
881909
setenv_var TESTSUITE_ABORT_ON_ERROR depre_unload_bad
882910

883911
set ts_depre_unload_bad [msg_unload err/1.0 [msg_moderr {invalid command name "bad"} bad $mp/err/1.0 2 {} {} {} $custom_error_trace]]\n\n

0 commit comments

Comments
 (0)