memory leak (1): unset private parameter

memory leak (1): unset private parameter

[Jun T]

valgrind found two memory leaks for which I don't have any quick fix.

This post is for the 1st problem (another post will follow for the 2nd).

% cat unset-private
() {
	private foo
	unset foo
}
% valgrind --leak-check=full zsh -f unset-private

32 bytes in 1 blocks are definitely lost in loss record 247 of 387
  by 0x1935B9: zalloc (mem.c:966)
  by 0x50B0772: makeprivate (param_private.c:138)
  by 0x165D08: scanmatchtable (hashtable.c:433)
  by 0x165D9E: scanhashtable (hashtable.c:449)
  by 0x50B0D7A: bin_private (param_private.c:248)

param_private.c:138 is
    struct gsu_closure *gsu = zalloc(sizeof(struct gsu_closure));

and this 'gsu' is saved in pm->gsu.s.

'unset foo' calls pps_unsetfn(pm, explicit=1), in param_private.c:

314     if (locallevel <= pm->level)
315         gsu->unsetfn(pm, explicit);		# this sets PM_UNSET
316     if (explicit) {
317         pm->node.flags |= PM_DECLARED;
318         pm->gsu.s = (GsuScalar)c;
319     } else
320         zfree(c, sizeof(struct gsu_closure)); # NOT called

When exiting from the function, endparmscope() (indirectly) calls
unsetpm_pm(pm,, explicit=0), but it does not call
pps_unsetfn(pm, expllicit=0) since PM_UNSET is already set
(params.c:3793), and the gsu_closure is not freed.

Maybe we need to set some info in pm to inform unsetpm_pm(pm)
that gsu need be freed (but there is no flag PM_PRIVATE).

Re: memory leak (1): unset private parameter

[Bart Schaefer]

On Fri, Jun 28, 2024 at 3:05 AM Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
>
> 32 bytes in 1 blocks are definitely lost in loss record 247 of 387
>   by 0x1935B9: zalloc (mem.c:966)
>   by 0x50B0772: makeprivate (param_private.c:138)
>
> 320         zfree(c, sizeof(struct gsu_closure)); # NOT called

Is that ever called for a private param?  If not, given that private
is always locally scoped, perhaps it would work for makeprivate() to
put the GSU on the heap.

Re: memory leak (1): unset private parameter

[Bart Schaefer]

On Fri, Jun 28, 2024 at 9:55 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> ... perhaps it would work for makeprivate() to
> put the GSU on the heap.

No such luck.

Re: memory leak (1): unset private parameter

[Bart Schaefer]

On Fri, Jun 28, 2024 at 3:05 AM Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
>
> When exiting from the function, endparmscope() (indirectly) calls
> unsetpm_pm(pm,, explicit=0), but it does not call
> pps_unsetfn(pm, expllicit=0) since PM_UNSET is already set
> (params.c:3793), and the gsu_closure is not freed.
>
> Maybe we need to set some info in pm to inform unsetpm_pm(pm)
> that gsu need be freed (but there is no flag PM_PRIVATE).

Seems to be sufficient to test PM_REMOVABLE; this sort of makes sense,
as removing the special from the parameter table conceivably requires
calling the GSU.

diff --git a/Src/params.c b/Src/params.c
index f65aa1e80..83bdb785d 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -3790,7 +3790,7 @@ unsetparam_pm(Param pm, int altflag, int exp)
        altremove = NULL;

     pm->node.flags &= ~PM_DECLARED;    /* like ksh, not like bash */
-    if (!(pm->node.flags & PM_UNSET))
+    if (!(pm->node.flags & PM_UNSET) || (pm->node.flags & PM_REMOVABLE))
        pm->gsu.s->unsetfn(pm, exp);
     if (pm->env)
        delenv(pm);