[vim/vim] Fix memory leak in `list_extend_func()` in `src/list.c` (PR #19572)

8 views
Skip to first unread message

Huihui Huang

unread,
Mar 3, 2026, 11:09:14 PM (3 days ago) Mar 3
to vim/vim, Subscribed

Problem

In list_extend_func(), when is_new is true, a new list is created using list_copy() (lines 3017–3022):

if (is_new)
{
    l1 = list_copy(l1, FALSE, TRUE, get_copyID());
    if (l1 == NULL)
        return;
}

After this allocation, several error paths may return early:

  • When the third argument has a type error (lines 3026–3028):
before = (long)tv_get_number_chk(&argvars[2], &error);
if (error)
    return;  // type error; errmsg already given
  • When list_find() fails (lines 3034–3039):
item = list_find(l1, before);
if (item == NULL)
{
    semsg(_(e_list_index_out_of_range_nr), before);
    return;
}
  • When check_typval_arg_type() fails (lines 3044–3046):
if (type != NULL && check_typval_arg_type(
        type, &argvars[1], func_name, 2) == FAIL)
    return;

If is_new is true, l1 refers to the newly allocated list. Returning early without releasing it causes the copied list to leak.

Solution

Release the copied list before returning on these error paths when is_new is true. The fix is included in this commit.


You can view, comment on, or merge this pull request online at:

  https://github.com/vim/vim/pull/19572

Commit Summary

  • 5900b67 Fix: memory leak in list_extend_func() in src/list.c

File Changes

(1 file)

Patch Links:


Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/19572@github.com>

Christian Brabandt

unread,
Mar 4, 2026, 1:55:16 PM (2 days ago) Mar 4
to vim/vim, Subscribed
chrisbra left a comment (vim/vim#19572)

thanks


Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/19572/c3999509266@github.com>

Christian Brabandt

unread,
Mar 4, 2026, 2:07:37 PM (2 days ago) Mar 4
to vim/vim, Subscribed
chrisbra left a comment (vim/vim#19572)

You are repeating the code. How about this change instead;

diff --git a/src/list.c b/src/list.c
index 310d8516b..f7b77602d 100644
--- a/src/list.c
+++ b/src/list.c
@@ -3025,7 +3025,7 @@ list_extend_func(
        {
            before = (long)tv_get_number_chk(&argvars[2], &error);
            if (error)
-               return;         // type error; errmsg already given
+               goto cleanup;   // type error; errmsg already given

            if (before == l1->lv_len)
                item = NULL;
@@ -3035,7 +3035,7 @@ list_extend_func(
                if (item == NULL)
                {
                    semsg(_(e_list_index_out_of_range_nr), before);
-                   return;
+                   goto cleanup;
                }
            }
        }
@@ -3043,7 +3043,7 @@ list_extend_func(
            item = NULL;
        if (type != NULL && check_typval_arg_type(
                    type, &argvars[1], func_name, 2) == FAIL)
-           return;
+           goto cleanup;
        list_extend(l1, l2, item);

        if (is_new)
@@ -3054,6 +3054,11 @@ list_extend_func(
        }
        else
            copy_tv(&argvars[0], rettv);
+       return;
+
+cleanup:
+       if (is_new)
+           list_unref(l1);
     }
 }


Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/19572/c3999586840@github.com>

Paul Ollis

unread,
12:32 PM (10 hours ago) 12:32 PM
to vim/vim, Subscribed
paul-ollis left a comment (vim/vim#19572)

I though I should just point out that in this case and a number of other recent similar PRs that, strictly speaking, memory is not being leaked. The lists and dictionaries involved are Vim script variables which (eventually) get tracked and cleaned up by Vim's garbage collector.

I am not sure, but I suspect that some code has been written with the implicit assumption that garbage collection would handle such cases.

I mention this, in case it has not already been considered. My personal opinion is that garbage collection should not be relied upon in such cases.


Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/19572/c4013048885@github.com>

zeertzjq

unread,
5:08 PM (5 hours ago) 5:08 PM
to vim/vim, Subscribed
zeertzjq left a comment (vim/vim#19572)

I think that only happens when the reference count is zero. Otherwise there will be a "leak" that isn't detectable by ASAN as the garbage collector still tracks all the lists and dicts.


Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/19572/c4014419433@github.com>

Paul Ollis

unread,
5:43 PM (5 hours ago) 5:43 PM
to vim/vim, Subscribed
paul-ollis left a comment (vim/vim#19572)

@zeetzjq The reference count is used to immediately free Vim variables when, for example, one is removed from a list of dictionary. And in most cases that is what happens.

The garbage collector identifies unreachable lists and dictionaries and frees them, regardless of their reference count.


Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/19572/c4014554467@github.com>

zeertzjq

unread,
5:59 PM (4 hours ago) 5:59 PM
to vim/vim, Subscribed
zeertzjq left a comment (vim/vim#19572)

Hmm, my previous experience indicates that's not always the case: #17980


Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/19572/c4014624152@github.com>

Reply all
Reply to author
Forward
0 new messages