From 4c76b1e981f072229944a22e5d5ee76fe42d994a Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Wed, 9 Oct 2019 19:40:50 +0200 Subject: Test and initial fix for crash with dictwatcherdel Fixes https://github.com/neovim/neovim/issues/11188. --- src/nvim/eval/typval.c | 10 +++++++++- src/nvim/eval/typval.h | 1 + 2 files changed, 10 insertions(+), 1 deletion(-) (limited to 'src/nvim/eval') diff --git a/src/nvim/eval/typval.c b/src/nvim/eval/typval.c index 9be487f4fd..f8b48d0139 100644 --- a/src/nvim/eval/typval.c +++ b/src/nvim/eval/typval.c @@ -1109,6 +1109,7 @@ void tv_dict_watcher_add(dict_T *const dict, const char *const key_pattern, watcher->key_pattern_len = key_pattern_len; watcher->callback = callback; watcher->busy = false; + watcher->needs_free = false; QUEUE_INSERT_TAIL(&dict->watchers, &watcher->node); } @@ -1197,7 +1198,11 @@ bool tv_dict_watcher_remove(dict_T *const dict, const char *const key_pattern, } QUEUE_REMOVE(w); - tv_dict_watcher_free(watcher); + if (watcher->busy) { + watcher->needs_free = true; + } else { + tv_dict_watcher_free(watcher); + } return true; } @@ -1268,6 +1273,9 @@ void tv_dict_watcher_notify(dict_T *const dict, const char *const key, callback_call(&watcher->callback, 3, argv, &rettv); watcher->busy = false; tv_clear(&rettv); + if (watcher->needs_free) { + tv_dict_watcher_free(watcher); + } } } tv_dict_unref(dict); diff --git a/src/nvim/eval/typval.h b/src/nvim/eval/typval.h index 531b17cb59..2b4612016b 100644 --- a/src/nvim/eval/typval.h +++ b/src/nvim/eval/typval.h @@ -89,6 +89,7 @@ typedef struct dict_watcher { size_t key_pattern_len; QUEUE node; bool busy; // prevent recursion if the dict is changed in the callback + bool needs_free; } DictWatcher; /// Bool variable values -- cgit From 36caafeb281bf872f11d475e594eb212636daa4d Mon Sep 17 00:00:00 2001 From: erw7 Date: Wed, 16 Oct 2019 16:23:07 +0200 Subject: Change QUEUE_FOREACH macro to use while instead of for --- src/nvim/eval/typval.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/nvim/eval') diff --git a/src/nvim/eval/typval.c b/src/nvim/eval/typval.c index f8b48d0139..5b3e31b324 100644 --- a/src/nvim/eval/typval.c +++ b/src/nvim/eval/typval.c @@ -1183,7 +1183,7 @@ bool tv_dict_watcher_remove(dict_T *const dict, const char *const key_pattern, QUEUE *w = NULL; DictWatcher *watcher = NULL; bool matched = false; - QUEUE_FOREACH(w, &dict->watchers) { + QUEUE_FOREACH(w, &dict->watchers, { watcher = tv_dict_watcher_node_data(w); if (tv_callback_equal(&watcher->callback, &callback) && watcher->key_pattern_len == key_pattern_len @@ -1191,7 +1191,7 @@ bool tv_dict_watcher_remove(dict_T *const dict, const char *const key_pattern, matched = true; break; } - } + }) if (!matched) { return false; @@ -1265,7 +1265,7 @@ void tv_dict_watcher_notify(dict_T *const dict, const char *const key, dict->dv_refcount++; QUEUE *w; - QUEUE_FOREACH(w, &dict->watchers) { + QUEUE_FOREACH(w, &dict->watchers, { DictWatcher *watcher = tv_dict_watcher_node_data(w); if (!watcher->busy && tv_dict_watcher_matches(watcher, key)) { rettv = TV_INITIAL_VALUE; @@ -1277,7 +1277,7 @@ void tv_dict_watcher_notify(dict_T *const dict, const char *const key, tv_dict_watcher_free(watcher); } } - } + }) tv_dict_unref(dict); for (size_t i = 1; i < ARRAY_SIZE(argv); i++) { -- cgit From 7268d49c50d01a841a32856cfc5713ae8de2fc96 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 29 Nov 2019 09:42:58 +0100 Subject: tv_dict_watcher_remove: delay freeing with busy queue --- src/nvim/eval/typval.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'src/nvim/eval') diff --git a/src/nvim/eval/typval.c b/src/nvim/eval/typval.c index 5b3e31b324..726619eb43 100644 --- a/src/nvim/eval/typval.c +++ b/src/nvim/eval/typval.c @@ -1183,8 +1183,12 @@ bool tv_dict_watcher_remove(dict_T *const dict, const char *const key_pattern, QUEUE *w = NULL; DictWatcher *watcher = NULL; bool matched = false; + bool queue_is_busy = false; QUEUE_FOREACH(w, &dict->watchers, { watcher = tv_dict_watcher_node_data(w); + if (watcher->busy) { + queue_is_busy = true; + } if (tv_callback_equal(&watcher->callback, &callback) && watcher->key_pattern_len == key_pattern_len && memcmp(watcher->key_pattern, key_pattern, key_pattern_len) == 0) { @@ -1197,10 +1201,10 @@ bool tv_dict_watcher_remove(dict_T *const dict, const char *const key_pattern, return false; } - QUEUE_REMOVE(w); - if (watcher->busy) { + if (queue_is_busy) { watcher->needs_free = true; } else { + QUEUE_REMOVE(w); tv_dict_watcher_free(watcher); } return true; @@ -1273,9 +1277,13 @@ void tv_dict_watcher_notify(dict_T *const dict, const char *const key, callback_call(&watcher->callback, 3, argv, &rettv); watcher->busy = false; tv_clear(&rettv); - if (watcher->needs_free) { - tv_dict_watcher_free(watcher); - } + } + }) + QUEUE_FOREACH(w, &dict->watchers, { + DictWatcher *watcher = tv_dict_watcher_node_data(w); + if (watcher->needs_free) { + QUEUE_REMOVE(w); + tv_dict_watcher_free(watcher); } }) tv_dict_unref(dict); -- cgit From d6cac809b009e7066c8761b1466fe08378146d22 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Sat, 30 Nov 2019 02:06:16 +0100 Subject: tv_dict_watcher_notify: any_needs_free --- src/nvim/eval/typval.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'src/nvim/eval') diff --git a/src/nvim/eval/typval.c b/src/nvim/eval/typval.c index 726619eb43..fe3d147040 100644 --- a/src/nvim/eval/typval.c +++ b/src/nvim/eval/typval.c @@ -1267,6 +1267,7 @@ void tv_dict_watcher_notify(dict_T *const dict, const char *const key, typval_T rettv; + bool any_needs_free = false; dict->dv_refcount++; QUEUE *w; QUEUE_FOREACH(w, &dict->watchers, { @@ -1277,15 +1278,20 @@ void tv_dict_watcher_notify(dict_T *const dict, const char *const key, callback_call(&watcher->callback, 3, argv, &rettv); watcher->busy = false; tv_clear(&rettv); + if (watcher->needs_free) { + any_needs_free = true; + } } }) - QUEUE_FOREACH(w, &dict->watchers, { - DictWatcher *watcher = tv_dict_watcher_node_data(w); - if (watcher->needs_free) { - QUEUE_REMOVE(w); - tv_dict_watcher_free(watcher); - } - }) + if (any_needs_free) { + QUEUE_FOREACH(w, &dict->watchers, { + DictWatcher *watcher = tv_dict_watcher_node_data(w); + if (watcher->needs_free) { + QUEUE_REMOVE(w); + tv_dict_watcher_free(watcher); + } + }) + } tv_dict_unref(dict); for (size_t i = 1; i < ARRAY_SIZE(argv); i++) { -- cgit