diff options
author | Justin M. Keyes <justinkz@gmail.com> | 2018-03-20 23:57:41 +0100 |
---|---|---|
committer | Justin M. Keyes <justinkz@gmail.com> | 2018-03-24 11:01:24 +0100 |
commit | 189c5abeba4fb508d879ebbf5fa07965c4092cf2 (patch) | |
tree | 5b96ee3b3c3e1d73606c19e1e8b8b4a8904b50c0 | |
parent | 96273256843ea357d62696ef307d6610ba97334c (diff) | |
download | rneovim-189c5abeba4fb508d879ebbf5fa07965c4092cf2.tar.gz rneovim-189c5abeba4fb508d879ebbf5fa07965c4092cf2.tar.bz2 rneovim-189c5abeba4fb508d879ebbf5fa07965c4092cf2.zip |
provider/RPC: apply_autocmds_group(): fix double-free
During provider dispatch, eval_call_provider() saves global
state--including pointers, such as `autocmd_fname`--into
`provider_caller_scope` which is later restored by f_rpcrequest().
But `autocmd_fname` is special-cased in eval_vars(), for performance
(see Vim patch 7.2.021; this is also the singular purpose of the
`autocmd_fname_full` global. Yay!)
If eval_vars() frees `autocmd_fname` then its provider-RPC-scoped alias
becomes a problem.
Solution: Don't free autocmd_fname in eval_vars(), just copy into it.
closes #5245
closes #5617
Reference
------------------------------------------------------------------------
Vim patch 7.2.021
https://github.com/vim/vim/commit/f6dad43c98f47da1ff9d8c99b320fc3674f83c63
Problem: When executing autocommands getting the full file name may be
slow. (David Kotchan)
Solution: Postpone calling FullName_save() until autocmd_fname is used.
vim_dev discussion (2008): "Problem with CursorMoved AutoCommand when
Editing Files on a Remote WIndows Share"
https://groups.google.com/d/msg/vim_dev/kj95weZa_eE/GTgj4aq5sIgJ
-rw-r--r-- | src/nvim/eval.c | 5 | ||||
-rw-r--r-- | src/nvim/ex_docmd.c | 17 | ||||
-rw-r--r-- | src/nvim/fileio.c | 3 | ||||
-rw-r--r-- | test/functional/provider/python3_spec.lua | 19 | ||||
-rw-r--r-- | test/functional/provider/ruby_spec.lua | 22 |
5 files changed, 48 insertions, 18 deletions
diff --git a/src/nvim/eval.c b/src/nvim/eval.c index feccec85c6..2b4df14d78 100644 --- a/src/nvim/eval.c +++ b/src/nvim/eval.c @@ -13828,7 +13828,7 @@ static void f_rpcrequest(typval_T *argvars, typval_T *rettv, FunPtr fptr) save_autocmd_fname_full = autocmd_fname_full; save_autocmd_bufnr = autocmd_bufnr; save_funccalp = save_funccal(); - // + current_SID = provider_caller_scope.SID; sourcing_name = provider_caller_scope.sourcing_name; sourcing_lnum = provider_caller_scope.sourcing_lnum; @@ -22481,7 +22481,8 @@ typval_T eval_call_provider(char *provider, char *method, list_T *arguments) restore_funccal(provider_caller_scope.funccalp); provider_caller_scope = saved_provider_caller_scope; provider_call_nesting--; - + assert(provider_call_nesting >= 0); + return rettv; } diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c index e1d28b4a9c..51f11b7b56 100644 --- a/src/nvim/ex_docmd.c +++ b/src/nvim/ex_docmd.c @@ -8547,15 +8547,16 @@ eval_vars ( break; case SPEC_AFILE: /* file name for autocommand */ - result = autocmd_fname; - if (result != NULL && !autocmd_fname_full) { - /* Still need to turn the fname into a full path. It is - * postponed to avoid a delay when <afile> is not used. */ - autocmd_fname_full = TRUE; - result = (char_u *)FullName_save((char *)autocmd_fname, FALSE); - xfree(autocmd_fname); - autocmd_fname = result; + if (autocmd_fname != NULL && !autocmd_fname_full) { + // Still need to turn the fname into a full path. It was + // postponed to avoid a delay when <afile> is not used. + autocmd_fname_full = true; + result = (char_u *)FullName_save((char *)autocmd_fname, false); + // Copy into `autocmd_fname`, don't reassign it. #8165 + xstrlcpy((char *)autocmd_fname, (char *)result, MAXPATHL); + xfree(result); } + result = autocmd_fname; if (result == NULL) { *errormsg = (char_u *)_( "E495: no autocommand file name to substitute for \"<afile>\""); diff --git a/src/nvim/fileio.c b/src/nvim/fileio.c index 91b0a695f1..4e70daea90 100644 --- a/src/nvim/fileio.c +++ b/src/nvim/fileio.c @@ -6755,7 +6755,8 @@ static bool apply_autocmds_group(event_T event, char_u *fname, char_u *fname_io, autocmd_fname = fname_io; } if (autocmd_fname != NULL) { - autocmd_fname = vim_strsave(autocmd_fname); + // Allocate MAXPATHL for when eval_vars() resolves the fullpath. + autocmd_fname = vim_strnsave(autocmd_fname, MAXPATHL); } autocmd_fname_full = false; // call FullName_save() later diff --git a/test/functional/provider/python3_spec.lua b/test/functional/provider/python3_spec.lua index f06728ec0e..93ac3ae017 100644 --- a/test/functional/provider/python3_spec.lua +++ b/test/functional/provider/python3_spec.lua @@ -3,6 +3,7 @@ local eval, command, feed = helpers.eval, helpers.command, helpers.feed local eq, clear, insert = helpers.eq, helpers.clear, helpers.insert local expect, write_file = helpers.expect, helpers.write_file local feed_command = helpers.feed_command +local source = helpers.source local missing_provider = helpers.missing_provider do @@ -13,7 +14,7 @@ do end end -describe('python3 commands and functions', function() +describe('python3 provider', function() before_each(function() clear() command('python3 import vim') @@ -82,4 +83,20 @@ describe('python3 commands and functions', function() it('py3eval', function() eq({1, 2, {['key'] = 'val'}}, eval([[py3eval('[1, 2, {"key": "val"}]')]])) end) + + it('RPC call to expand("<afile>") during BufDelete #5245 #5617', function() + source([=[ + python3 << EOF + import vim + def foo(): + vim.eval('expand("<afile>:p")') + vim.eval('bufnr(expand("<afile>:p"))') + EOF + autocmd BufDelete * python3 foo() + autocmd BufUnload * python3 foo()]=]) + feed_command("exe 'split' tempname()") + feed_command("bwipeout!") + feed_command('help help') + eq(2, eval('1+1')) -- Still alive? + end) end) diff --git a/test/functional/provider/ruby_spec.lua b/test/functional/provider/ruby_spec.lua index a2c6c6a10e..e049ac7c41 100644 --- a/test/functional/provider/ruby_spec.lua +++ b/test/functional/provider/ruby_spec.lua @@ -1,16 +1,18 @@ local helpers = require('test.functional.helpers')(after_each) +local clear = helpers.clear +local command = helpers.command +local curbufmeths = helpers.curbufmeths local eq = helpers.eq +local eval = helpers.eval +local expect = helpers.expect local feed = helpers.feed -local clear = helpers.clear +local feed_command = helpers.feed_command local funcs = helpers.funcs -local meths = helpers.meths local insert = helpers.insert -local expect = helpers.expect -local command = helpers.command -local write_file = helpers.write_file -local curbufmeths = helpers.curbufmeths +local meths = helpers.meths local missing_provider = helpers.missing_provider +local write_file = helpers.write_file do clear() @@ -90,3 +92,11 @@ describe(':rubydo command', function() eq(false, curbufmeths.get_option('modified')) end) end) + +describe('ruby provider', function() + it('RPC call to expand("<afile>") during BufDelete #5245 #5617', function() + command([=[autocmd BufDelete * ruby VIM::evaluate('expand("<afile>")')]=]) + feed_command('help help') + eq(2, eval('1+1')) -- Still alive? + end) +end) |