patch 9.1.0999: Vim9: leaking finished exception
Commit:
https://github.com/vim/vim/commit/2051af1642843426714efc2572c3e270fe0948be
Author: Yee Cheng Chin <[email protected]>
Date: Thu Jan 9 22:14:34 2025 +0100
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 <[email protected]>
Signed-off-by: Christian Brabandt <[email protected]>
diff --git a/src/ex_eval.c b/src/ex_eval.c
index e996ce2a1..3c865c396 100644
--- a/src/ex_eval.c
+++ b/src/ex_eval.c
@@ -718,7 +718,7 @@ catch_exception(except_T *excp)
/*
* Remove an exception from the caught stack.
*/
- static void
+ void
finish_exception(except_T *excp)
{
if (excp != caught_stack)
diff --git a/src/proto/ex_eval.pro b/src/proto/ex_eval.pro
index 979d6fb8f..feffc8fb0 100644
--- a/src/proto/ex_eval.pro
+++ b/src/proto/ex_eval.pro
@@ -11,6 +11,7 @@ char *get_exception_string(void *value, except_type_T type,
char_u *cmdname, int
int throw_exception(void *value, except_type_T type, char_u *cmdname);
void discard_current_exception(void);
void catch_exception(except_T *excp);
+void finish_exception(except_T *excp);
void exception_state_save(exception_state_T *estate);
void exception_state_restore(exception_state_T *estate);
void exception_state_clear(void);
diff --git a/src/testdir/test_stacktrace.vim b/src/testdir/test_stacktrace.vim
index 5c71d5023..fc8510a2d 100644
--- a/src/testdir/test_stacktrace.vim
+++ b/src/testdir/test_stacktrace.vim
@@ -10,7 +10,7 @@ func Filepath(name)
endfunc
func AssertStacktrace(expect, actual)
- call assert_equal(#{lnum: 617, filepath: Filepath('runtest.vim')},
a:actual[0])
+ call assert_equal(Filepath('runtest.vim'), a:actual[0]['filepath'])
call assert_equal(a:expect, a:actual[-len(a:expect):])
endfunc
@@ -97,6 +97,12 @@ func Test_vstacktrace()
call Xfunc1()
catch
let stacktrace = v:stacktrace
+ try
+ call Xfunc1()
+ catch
+ let stacktrace_inner = v:stacktrace
+ endtry
+ let stacktrace_after = v:stacktrace " should be restored by the exception
stack to the previous one
endtry
call assert_equal([], v:stacktrace)
call AssertStacktrace([
@@ -104,9 +110,15 @@ func Test_vstacktrace()
\ #{funcref: funcref('Xfunc1'), lnum: 5, filepath:
Filepath('Xscript1')},
\ #{funcref: funcref('Xfunc2'), lnum: 4, filepath:
Filepath('Xscript2')},
\ ], stacktrace)
+ call AssertStacktrace([
+ \ #{funcref: funcref('Test_vstacktrace'), lnum: 101, filepath:
s:thisfile},
+ \ #{funcref: funcref('Xfunc1'), lnum: 5, filepath:
Filepath('Xscript1')},
+ \ #{funcref: funcref('Xfunc2'), lnum: 4, filepath:
Filepath('Xscript2')},
+ \ ], stacktrace_inner)
+ call assert_equal(stacktrace, stacktrace_after)
endfunc
-func Test_zzz_stacktrace_vim9()
+func Test_stacktrace_vim9()
let lines =<< trim [SCRIPT]
var stacktrace = getstacktrace()
assert_notequal([], stacktrace)
@@ -122,11 +134,9 @@ func Test_zzz_stacktrace_vim9()
assert_true(has_key(d, 'lnum'))
endfor
endtry
+ call assert_equal([], v:stacktrace)
[SCRIPT]
call v9.CheckDefSuccess(lines)
- " FIXME: v:stacktrace is not cleared after the exception handling, and this
- " test has to be run as the last one because of this.
- " call assert_equal([], v:stacktrace)
endfunc
" vim: shiftwidth=2 sts=2 expandtab
diff --git a/src/testdir/test_vim9_script.vim b/src/testdir/test_vim9_script.vim
index 550c725dd..b29f3cd80 100644
--- a/src/testdir/test_vim9_script.vim
+++ b/src/testdir/test_vim9_script.vim
@@ -945,6 +945,47 @@ def Test_try_catch_throw()
endif
END
v9.CheckDefAndScriptSuccess(lines)
+
+ # test that the v:exception stacks are correctly restored
+ try
+ try
+ throw 101
+ catch
+ assert_equal('101', v:exception)
+ try
+ catch
+ finally
+ assert_equal('101', v:exception) # finally shouldn't clear if it
doesn't own it
+ endtry
+ assert_equal('101', v:exception)
+ throw 102 # Re-throw inside catch block
+ endtry
+ catch
+ assert_equal('102', v:exception)
+ try
+ throw 103 # throw inside nested exception stack
+ catch
+ assert_equal('103', v:exception)
+ endtry
+ assert_equal('102', v:exception) # restored stack
+ finally
+ assert_equal('', v:exception) # finally should clear if it owns the
exception
+ endtry
+ try
+ try
+ throw 104
+ catch
+ try
+ exec 'nonexistent_cmd' # normal exception inside nested exception stack
+ catch
+ assert_match('E492:', v:exception)
+ endtry
+ eval [][0] # normal exception inside catch block
+ endtry
+ catch
+ assert_match('E684:', v:exception)
+ endtry
+ assert_equal('', v:exception) # All exceptions properly popped
enddef
def Test_unreachable_after()
@@ -1417,11 +1458,23 @@ def Test_throw_line_number()
eval 2 + 2
throw 'exception'
enddef
+ def Func2()
+ eval 1 + 1
+ eval 2 + 2
+ eval 3 + 3
+ throw 'exception'
+ enddef
try
Func()
catch /exception/
+ try
+ Func2()
+ catch /exception/
+ assert_match('line 4', v:throwpoint)
+ endtry
assert_match('line 3', v:throwpoint)
endtry
+ assert_match('', v:throwpoint)
enddef
diff --git a/src/version.c b/src/version.c
index 49a3523a2..46b7e3559 100644
--- a/src/version.c
+++ b/src/version.c
@@ -704,6 +704,8 @@ static char *(features[]) =
static int included_patches[] =
{ /* Add new patch number below this line */
+/**/
+ 999,
/**/
998,
/**/
diff --git a/src/vim9execute.c b/src/vim9execute.c
index d6962804b..c7f0e673b 100644
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -3281,7 +3281,15 @@ exec_instructions(ectx_T *ectx)
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 @@ exec_instructions(ectx_T *ectx)
// 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 @@ exec_instructions(ectx_T *ectx)
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 @@ exec_instructions(ectx_T *ectx)
{
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;
}
}
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/vim_dev/E1tW06N-001CKC-QB%40256bit.org.
