On Sunday, June 22, 2014 3:29:32 PM UTC-5, Ben Fritz wrote: > The attached patch seems to fix the crash reported here: > > https://groups.google.com/d/topic/vim_dev/Nr8Ja4Zjghw/discussion >
I missed a couple files in the first patch; updated patch attached. I still need help with the LUA interface. > > First, if I don't use :unlet dict list, but instead source the script > again and allow the previous values to be garbage collected, Vim stays > busy for at least an hour. I don't know if it ever finishes because I > killed the process. I used gdb on MinGW to see that Vim DOES get > through the "set reference" calls in the garbage collector which my > patch fixed; so I expect Vim is busy with the recursive calls to > actually free the garbage-collected memory. These could probably be > fixed in the same way I fixed the reference setting recursion. I think > this deserves a separate patch. > Unfortunately I tried the same approach with the list_free and dict_free chains and there did not seem to be a usable impact on performance. I let it run for half an hour and Vim was still busy...but it did finally finish sometime after I went to bed, so I guess there is that. -- -- 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]. For more options, visit https://groups.google.com/d/optout.
# HG changeset patch # User Ben Fritz <[email protected]> # Date 1403502771 18000 # Mon Jun 23 00:52:51 2014 -0500 # Branch 2html-dev # Node ID adb9502d50c1e5796a2af1162c33784010410e12 # Parent 0560ce8d74895ba362a9a812342d22c4bf18e5c9 Make the early part of garbage collect be iterative to solve crashing problems diff -r 0560ce8d7489 -r adb9502d50c1 src/eval.c --- a/src/eval.c Sun Jun 15 23:08:06 2014 -0500 +++ b/src/eval.c Mon Jun 23 00:52:51 2014 -0500 @@ -93,7 +93,6 @@ char_u *ll_newkey; /* New key for Dict in alloc. mem or NULL. */ } lval_T; - static char *e_letunexp = N_("E18: Unexpected characters in :let"); static char *e_listidx = N_("E684: list index out of range: %ld"); static char *e_undefvar = N_("E121: Undefined variable: %s"); @@ -6775,6 +6774,7 @@ garbage_collect() { int copyID; + int abort = 0; buf_T *buf; win_T *wp; int i; @@ -6805,82 +6805,85 @@ * the item is referenced elsewhere the funccal must not be freed. */ for (fc = previous_funccal; fc != NULL; fc = fc->caller) { - set_ref_in_ht(&fc->l_vars.dv_hashtab, copyID + 1); - set_ref_in_ht(&fc->l_avars.dv_hashtab, copyID + 1); + abort = abort || set_ref_in_ht(&fc->l_vars.dv_hashtab, copyID + 1, NULL); + abort = abort || set_ref_in_ht(&fc->l_avars.dv_hashtab, copyID + 1, NULL); } /* script-local variables */ for (i = 1; i <= ga_scripts.ga_len; ++i) - set_ref_in_ht(&SCRIPT_VARS(i), copyID); + abort = abort || set_ref_in_ht(&SCRIPT_VARS(i), copyID, NULL); /* buffer-local variables */ for (buf = firstbuf; buf != NULL; buf = buf->b_next) - set_ref_in_item(&buf->b_bufvar.di_tv, copyID); + abort = abort || set_ref_in_item(&buf->b_bufvar.di_tv, copyID, NULL, NULL); /* window-local variables */ FOR_ALL_TAB_WINDOWS(tp, wp) - set_ref_in_item(&wp->w_winvar.di_tv, copyID); + abort = abort || set_ref_in_item(&wp->w_winvar.di_tv, copyID, NULL, NULL); #ifdef FEAT_AUTOCMD if (aucmd_win != NULL) - set_ref_in_item(&aucmd_win->w_winvar.di_tv, copyID); + abort = abort || set_ref_in_item(&aucmd_win->w_winvar.di_tv, copyID, NULL, NULL); #endif #ifdef FEAT_WINDOWS /* tabpage-local variables */ for (tp = first_tabpage; tp != NULL; tp = tp->tp_next) - set_ref_in_item(&tp->tp_winvar.di_tv, copyID); + abort = abort || set_ref_in_item(&tp->tp_winvar.di_tv, copyID, NULL, NULL); #endif /* global variables */ - set_ref_in_ht(&globvarht, copyID); + abort = abort || set_ref_in_ht(&globvarht, copyID, NULL); /* function-local variables */ for (fc = current_funccal; fc != NULL; fc = fc->caller) { - set_ref_in_ht(&fc->l_vars.dv_hashtab, copyID); - set_ref_in_ht(&fc->l_avars.dv_hashtab, copyID); + abort = abort || set_ref_in_ht(&fc->l_vars.dv_hashtab, copyID, NULL); + abort = abort || set_ref_in_ht(&fc->l_avars.dv_hashtab, copyID, NULL); } /* v: vars */ - set_ref_in_ht(&vimvarht, copyID); + abort = abort || set_ref_in_ht(&vimvarht, copyID, NULL); #ifdef FEAT_LUA set_ref_in_lua(copyID); #endif #ifdef FEAT_PYTHON - set_ref_in_python(copyID); + abort = abort || set_ref_in_python(copyID); #endif #ifdef FEAT_PYTHON3 - set_ref_in_python3(copyID); -#endif - - /* - * 2. Free lists and dictionaries that are not referenced. - */ - did_free = free_unref_items(copyID); - - /* - * 3. Check if any funccal can be freed now. - */ - for (pfc = &previous_funccal; *pfc != NULL; ) - { - if (can_free_funccal(*pfc, copyID)) - { - fc = *pfc; - *pfc = fc->caller; - free_funccal(fc, TRUE); - did_free = TRUE; - did_free_funccal = TRUE; - } - else - pfc = &(*pfc)->caller; - } - if (did_free_funccal) - /* When a funccal was freed some more items might be garbage - * collected, so run again. */ - (void)garbage_collect(); + abort = abort || set_ref_in_python3(copyID); +#endif + + if (!abort) + { + /* + * 2. Free lists and dictionaries that are not referenced. + */ + did_free = free_unref_items(copyID); + + /* + * 3. Check if any funccal can be freed now. + */ + for (pfc = &previous_funccal; *pfc != NULL; ) + { + if (can_free_funccal(*pfc, copyID)) + { + fc = *pfc; + *pfc = fc->caller; + free_funccal(fc, TRUE); + did_free = TRUE; + did_free_funccal = TRUE; + } + else + pfc = &(*pfc)->caller; + } + if (did_free_funccal) + /* When a funccal was freed some more items might be garbage + * collected, so run again. */ + (void)garbage_collect(); + } return did_free; } @@ -6940,48 +6943,113 @@ /* * Mark all lists and dicts referenced through hashtab "ht" with "copyID". - */ - void -set_ref_in_ht(ht, copyID) - hashtab_T *ht; - int copyID; + * + * Returns 0 if successful, 1 if setting references failed somehow. + */ + int +set_ref_in_ht(ht, copyID, list_stack) + hashtab_T *ht; + int copyID; + list_stack_T **list_stack; { int todo; + int abort = 0; hashitem_T *hi; - - todo = (int)ht->ht_used; - for (hi = ht->ht_array; todo > 0; ++hi) - if (!HASHITEM_EMPTY(hi)) - { - --todo; - set_ref_in_item(&HI2DI(hi)->di_tv, copyID); - } + hashtab_T *cur_ht = ht; + ht_stack_T *ht_stack = NULL; + ht_stack_T *tempitem; + + ht_stack = (ht_stack_T*)malloc(sizeof(ht_stack_T)); + if (ht_stack) + { + ht_stack->ht = ht; + ht_stack->prev = NULL; + } + else + { + abort = 1; + } + + while (ht_stack) + { + cur_ht = ht_stack->ht; + tempitem = ht_stack; + ht_stack = ht_stack->prev; + free(tempitem); + + if (!abort) + { + todo = (int)cur_ht->ht_used; + for (hi = cur_ht->ht_array; todo > 0; ++hi) + if (!HASHITEM_EMPTY(hi)) + { + --todo; + abort = set_ref_in_item(&HI2DI(hi)->di_tv, copyID, &ht_stack, list_stack); + } + } + } + + return abort; } /* * Mark all lists and dicts referenced through list "l" with "copyID". - */ - void -set_ref_in_list(l, copyID) + * + * Returns 0 if successful, 1 if setting references failed somehow. + */ + int +set_ref_in_list(l, copyID, ht_stack) list_T *l; int copyID; -{ - listitem_T *li; - - for (li = l->lv_first; li != NULL; li = li->li_next) - set_ref_in_item(&li->li_tv, copyID); + ht_stack_T **ht_stack; +{ + listitem_T *li; + int abort = 0; + + list_T *cur_l; + list_stack_T *list_stack = NULL; + list_stack_T *tempitem; + + list_stack = (list_stack_T*)malloc(sizeof(list_stack_T)); + if (list_stack) + { + list_stack->list = l; + list_stack->prev = NULL; + } + else + { + abort = 1; + } + + while (list_stack) + { + cur_l = list_stack->list; + tempitem = list_stack; + list_stack = list_stack->prev; + free(tempitem); + + for (li = cur_l->lv_first; !abort && li != NULL; li = li->li_next) + abort = set_ref_in_item(&li->li_tv, copyID, ht_stack, &list_stack); + } + + return abort; } /* * Mark all lists and dicts referenced through typval "tv" with "copyID". - */ - void -set_ref_in_item(tv, copyID) - typval_T *tv; - int copyID; + * + * Returns 0 if successful, 1 if setting references failed somehow. + */ + int +set_ref_in_item(tv, copyID, ht_stack, list_stack) + typval_T *tv; + int copyID; + ht_stack_T **ht_stack; + list_stack_T **list_stack; { dict_T *dd; list_T *ll; + int abort = 0; switch (tv->v_type) { @@ -6991,7 +7059,24 @@ { /* Didn't see this dict yet. */ dd->dv_copyID = copyID; - set_ref_in_ht(&dd->dv_hashtab, copyID); + if (NULL == ht_stack) + { + abort = set_ref_in_ht(&dd->dv_hashtab, copyID, list_stack); + } + else + { + ht_stack_T *newitem = (ht_stack_T*)malloc(sizeof(ht_stack_T)); + if (newitem) + { + newitem->ht = &dd->dv_hashtab; + newitem->prev = *ht_stack; + *ht_stack = newitem; + } + else + { + abort = 1; + } + } } break; @@ -7001,11 +7086,28 @@ { /* Didn't see this list yet. */ ll->lv_copyID = copyID; - set_ref_in_list(ll, copyID); - } - break; - } - return; + if (NULL == list_stack) + { + abort = set_ref_in_list(ll, copyID, ht_stack); + } + else + { + list_stack_T *newitem = (list_stack_T*)malloc(sizeof(list_stack_T)); + if (newitem) + { + newitem->list = ll; + newitem->prev = *list_stack; + *list_stack = newitem; + } + else + { + abort = 1; + } + } + } + break; + } + return abort; } /* diff -r 0560ce8d7489 -r adb9502d50c1 src/if_lua.c --- a/src/if_lua.c Sun Jun 15 23:08:06 2014 -0500 +++ b/src/if_lua.c Mon Jun 23 00:52:51 2014 -0500 @@ -1524,11 +1524,12 @@ luaV_setref (lua_State *L) { int copyID = lua_tointeger(L, 1); + int abort = 0; typval_T tv; luaV_getfield(L, LUAVIM_LIST); luaV_getfield(L, LUAVIM_DICT); lua_pushnil(L); - while (lua_next(L, lua_upvalueindex(1)) != 0) /* traverse cache table */ + while (!abort && lua_next(L, lua_upvalueindex(1)) != 0) /* traverse cache table */ { lua_getmetatable(L, -1); if (lua_rawequal(L, -1, 2)) /* list? */ @@ -1542,9 +1543,9 @@ tv.vval.v_dict = (dict_T *) lua_touserdata(L, 4); /* key */ } lua_pop(L, 2); /* metatable and value */ - set_ref_in_item(&tv, copyID); + abort = set_ref_in_item(&tv, copyID, NULL, NULL); } - return 0; + return abort; } static int diff -r 0560ce8d7489 -r adb9502d50c1 src/if_py_both.h --- a/src/if_py_both.h Sun Jun 15 23:08:06 2014 -0500 +++ b/src/if_py_both.h Mon Jun 23 00:52:51 2014 -0500 @@ -5495,34 +5495,41 @@ PyErr_Clear(); } - static void + static int set_ref_in_py(const int copyID) { pylinkedlist_T *cur; dict_T *dd; list_T *ll; + int abort = 0; if (lastdict != NULL) - for(cur = lastdict ; cur != NULL ; cur = cur->pll_prev) + { + for(cur = lastdict ; !abort && cur != NULL ; cur = cur->pll_prev) { dd = ((DictionaryObject *) (cur->pll_obj))->dict; if (dd->dv_copyID != copyID) { dd->dv_copyID = copyID; - set_ref_in_ht(&dd->dv_hashtab, copyID); + abort = set_ref_in_ht(&dd->dv_hashtab, copyID, NULL); } } + } if (lastlist != NULL) - for(cur = lastlist ; cur != NULL ; cur = cur->pll_prev) + { + for(cur = lastlist ; !abort && cur != NULL ; cur = cur->pll_prev) { ll = ((ListObject *) (cur->pll_obj))->list; if (ll->lv_copyID != copyID) { ll->lv_copyID = copyID; - set_ref_in_list(ll, copyID); + abort = set_ref_in_list(ll, copyID, NULL); } } + } + + return abort; } static int diff -r 0560ce8d7489 -r adb9502d50c1 src/if_python.c --- a/src/if_python.c Sun Jun 15 23:08:06 2014 -0500 +++ b/src/if_python.c Mon Jun 23 00:52:51 2014 -0500 @@ -1536,8 +1536,8 @@ } #endif /* Python 1.4 */ - void + int set_ref_in_python (int copyID) { - set_ref_in_py(copyID); + return set_ref_in_py(copyID); } diff -r 0560ce8d7489 -r adb9502d50c1 src/if_python3.c --- a/src/if_python3.c Sun Jun 15 23:08:06 2014 -0500 +++ b/src/if_python3.c Mon Jun 23 00:52:51 2014 -0500 @@ -1649,8 +1649,8 @@ } } - void + int set_ref_in_python3 (int copyID) { - set_ref_in_py(copyID); + int set_ref_in_py(copyID); } diff -r 0560ce8d7489 -r adb9502d50c1 src/proto/eval.pro --- a/src/proto/eval.pro Sun Jun 15 23:08:06 2014 -0500 +++ b/src/proto/eval.pro Mon Jun 23 00:52:51 2014 -0500 @@ -62,9 +62,9 @@ void vimlist_remove __ARGS((list_T *l, listitem_T *item, listitem_T *item2)); void list_insert __ARGS((list_T *l, listitem_T *ni, listitem_T *item)); int garbage_collect __ARGS((void)); -void set_ref_in_ht __ARGS((hashtab_T *ht, int copyID)); -void set_ref_in_list __ARGS((list_T *l, int copyID)); -void set_ref_in_item __ARGS((typval_T *tv, int copyID)); +int set_ref_in_ht __ARGS((hashtab_T *ht, int copyID, list_stack_T **list_stack)); +int set_ref_in_list __ARGS((list_T *l, int copyID, ht_stack_T **ht_stack)); +int set_ref_in_item __ARGS((typval_T *tv, int copyID, ht_stack_T **ht_stack, list_stack_T **list_stack)); dict_T *dict_alloc __ARGS((void)); void dict_unref __ARGS((dict_T *d)); void dict_free __ARGS((dict_T *d, int recurse)); diff -r 0560ce8d7489 -r adb9502d50c1 src/proto/if_python.pro --- a/src/proto/if_python.pro Sun Jun 15 23:08:06 2014 -0500 +++ b/src/proto/if_python.pro Mon Jun 23 00:52:51 2014 -0500 @@ -9,5 +9,5 @@ void python_window_free __ARGS((win_T *win)); void python_tabpage_free __ARGS((tabpage_T *tab)); void do_pyeval __ARGS((char_u *str, typval_T *rettv)); -void set_ref_in_python __ARGS((int copyID)); +int set_ref_in_python __ARGS((int copyID)); /* vim: set ft=c : */ diff -r 0560ce8d7489 -r adb9502d50c1 src/proto/if_python3.pro --- a/src/proto/if_python3.pro Sun Jun 15 23:08:06 2014 -0500 +++ b/src/proto/if_python3.pro Mon Jun 23 00:52:51 2014 -0500 @@ -9,5 +9,5 @@ void python3_window_free __ARGS((win_T *win)); void python3_tabpage_free __ARGS((tabpage_T *tab)); void do_py3eval __ARGS((char_u *str, typval_T *rettv)); -void set_ref_in_python3 __ARGS((int copyID)); +int set_ref_in_python3 __ARGS((int copyID)); /* vim: set ft=c : */ diff -r 0560ce8d7489 -r adb9502d50c1 src/structs.h --- a/src/structs.h Sun Jun 15 23:08:06 2014 -0500 +++ b/src/structs.h Mon Jun 23 00:52:51 2014 -0500 @@ -1217,6 +1217,20 @@ dict_T *dv_used_prev; /* previous dict in used dicts list */ }; +/* structure used for explicit stack while garbage collecting hash tables */ +typedef struct ht_stack_S +{ + hashtab_T *ht; + struct ht_stack_S *prev; +} ht_stack_T; + +/* structure used for explicit stack while garbage collecting lists */ +typedef struct list_stack_S +{ + list_T *list; + struct list_stack_S *prev; +} list_stack_T; + /* values for b_syn_spell: what to do with toplevel text */ #define SYNSPL_DEFAULT 0 /* spell check if @Spell not defined */ #define SYNSPL_TOP 1 /* spell check toplevel text */
