2015-11-05 1:22 GMT+03:00 Bram Moolenaar <[email protected]>:
>
> Nikolay Pavlov wrote:
>
>> Consider the following scenario: while processing function dict_copy
>>
>>         static dict_T *
>>     dict_copy(orig, deep, copyID)
>>         dict_T      *orig;
>>         int         deep;
>>         int         copyID;
>>     {
>>         dict_T      *copy;
>>         dictitem_T  *di;
>>         int         todo;
>>         hashitem_T  *hi;
>>
>>         if (orig == NULL)
>>             return NULL;
>>
>>         copy = dict_alloc();
>>
>> this allocation occupied last memory in the system,
>>
>>         if (copy != NULL)
>>         {
>>             if (copyID != 0)
>>             {
>>                 orig->dv_copyID = copyID;
>>                 orig->dv_copydict = copy;
>>             }
>>             todo = (int)orig->dv_hashtab.ht_used;
>>             for (hi = orig->dv_hashtab.ht_array; todo > 0 && !got_int; ++hi)
>>             {
>>                 if (!HASHITEM_EMPTY(hi))
>>                 {
>>                     --todo;
>>
>>                     di = dictitem_alloc(hi->hi_key);
>>
>> so that this allocation at first fails and results in
>> `garbage_collect()` being called from `lalloc()` in `misc2.c`.
>>
>> Since `copy` dictionary is not yet referenced anywhere
>> `garbage_collect()` call will free the dictionary and
>>
>>                     if (di == NULL)
>>                         break;
>>                     if (deep)
>>                     {
>>                         if (item_copy(&HI2DI(hi)->di_tv, &di->di_tv, deep,
>>
>> copyID) == FAIL)
>>                         {
>>                             vim_free(di);
>>                             break;
>>                         }
>>                     }
>>                     else
>>                         copy_tv(&HI2DI(hi)->di_tv, &di->di_tv);
>>                     if (dict_add(copy, di) == FAIL)
>>
>> Vim will crash at this point.
>>
>>                     {
>>                         dictitem_free(di);
>>                         break;
>>                     }
>>                 }
>>             }
>>
>>             ++copy->dv_refcount;
>>             if (todo > 0)
>>             {
>>                 dict_unref(copy);
>>                 copy = NULL;
>>             }
>>         }
>>
>>         return copy;
>>     }
>>
>> Please correct me if I am wrong.
>
> Looks like incrementing dv_refcount should happen much earlier.
>
> Real garbage collection is so much easier than reference counting...

I do not think this will work: `garbage_collect` is protecting against
memory leaks in cycles, so it will collect `copy` no matter what its
reference count is, it is enough that `copyID` will not be set.

Also note that this is only one of the examples: after identifying the
problem I just found a random function that has memory allocation
after `dict_alloc`, but before allocated dictionary is saved anywhere
where `garbage_collect` may assign it a copyID.

>
> --
> From "know your smileys":
>  !-|    I-am-a-Cylon-Centurian-with-one-red-eye-bouncing-back-and-forth
>
>  /// Bram Moolenaar -- [email protected] -- http://www.Moolenaar.net   \\\
> ///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
> \\\  an exciting new programming language -- http://www.Zimbu.org        ///
>  \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

-- 
-- 
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.

Raspunde prin e-mail lui