patch 9.1.0999: Vim9: leaking finished exception
Problem: leaking finished exception
(after v9.1.0984)
Solution: use finish_exception to clean up caught exceptions
(Yee Cheng Chin)
In Vimscript, v:exception/throwpoint/stacktrace are supposed to reflect
the currently caught exception, and be popped after the exception is
finished (via endtry, finally, or a thrown exception inside catch).
Vim9script does not handle this properly, and leaks them instead. This
is clearly visible when launching GVim with menu enabled. A caught
exception inside the s:BMShow() in menu.vim would show up when querying
`v:stacktrace` even though the exception was already caught and handled.
To fix this, just use the same functionality as Vimscript by calling
`finish_exception` to properly restore the states. Note that this
assumes `current_exception` is always the same as `caught_stack` which
believe should be the case.
Added tests for this. Also fix up test_stacktrace to properly test the
stack restore behavior where we have nested exceptions in catch blocks
and to also test the vim9script functionality properly.
- Also, remove its dependency on explicitly checking a line number in
runtest.vim which is a very fragile way to write tests as any minor
change in runtest.vim (shared among all tests) would require changing
test_stacktrace.vim. We don't actually need such granularity in the
test.
closes: #16413
Signed-off-by: Yee Cheng Chin <ychin.git@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
diff --git a/src/vim9execute.c b/src/vim9execute.c
index d696280..c7f0e67 100644
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -3281,7 +3281,15 @@
trycmd_T *trycmd = ((trycmd_T *)trystack->ga_data)
+ trystack->ga_len - 1;
if (trycmd->tcd_frame_idx == ectx->ec_frame_idx)
- trycmd->tcd_caught = FALSE;
+ {
+ if (trycmd->tcd_caught)
+ {
+ // Inside a "catch" we need to first discard the caught
+ // exception.
+ finish_exception(caught_stack);
+ trycmd->tcd_caught = FALSE;
+ }
+ }
}
}
@@ -4972,6 +4980,12 @@
// Reset the index to avoid a return statement jumps here
// again.
trycmd->tcd_finally_idx = 0;
+ if (trycmd->tcd_caught)
+ {
+ // discard the exception
+ finish_exception(caught_stack);
+ trycmd->tcd_caught = FALSE;
+ }
break;
}
@@ -4986,12 +5000,10 @@
trycmd = ((trycmd_T *)trystack->ga_data) + trystack->ga_len;
if (trycmd->tcd_did_throw)
did_throw = TRUE;
- if (trycmd->tcd_caught && current_exception != NULL)
+ if (trycmd->tcd_caught)
{
// discard the exception
- if (caught_stack == current_exception)
- caught_stack = caught_stack->caught;
- discard_current_exception();
+ finish_exception(caught_stack);
}
if (trycmd->tcd_return)
@@ -5040,12 +5052,10 @@
{
trycmd_T *trycmd = ((trycmd_T *)trystack->ga_data)
+ trystack->ga_len - 1;
- if (trycmd->tcd_caught && current_exception != NULL)
+ if (trycmd->tcd_caught)
{
// discard the exception
- if (caught_stack == current_exception)
- caught_stack = caught_stack->caught;
- discard_current_exception();
+ finish_exception(caught_stack);
trycmd->tcd_caught = FALSE;
}
}