Bug 1189294 - Leak on unset of associative array
Summary: Leak on unset of associative array
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: ksh
Version: 6.7
Hardware: Unspecified
OS: Unspecified
urgent
high
Target Milestone: rc
: ---
Assignee: Michal Hlavinka
QA Contact: Martin Kyral
URL:
Whiteboard:
Depends On: 1189297
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-02-04 22:13 UTC by Paulo Andrade
Modified: 2019-08-15 04:14 UTC (History)
5 users (show)

Fixed In Version: ksh-20120801-24.el6
Doc Type: Bug Fix
Doc Text:
Previously, after the user unset an associative array, the system did not free the newly-available memory. Consequently, ksh consumed more and more memory over time. The underlying source code has been modified to free the memory after the user unsets an associative array, thus fixing this problem.
Clone Of:
Environment:
Last Closed: 2015-07-22 06:56:34 UTC


Attachments (Terms of Use)
test3.ksh (705 bytes, application/x-shellscript)
2015-02-04 22:13 UTC, Paulo Andrade
no flags Details
ksh-20120801-assoc-unset-leak.patch (439 bytes, patch)
2015-02-04 22:15 UTC, Paulo Andrade
no flags Details | Diff
ksh-20120801-assoc-unset-leak.patch (545 bytes, patch)
2015-02-09 11:35 UTC, Paulo Andrade
no flags Details | Diff
ksh-20120801-assoc-unset-leak.patch (744 bytes, patch)
2015-02-10 20:51 UTC, Paulo Andrade
no flags Details | Diff


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2015:1450 normal SHIPPED_LIVE ksh bug fix update 2015-07-20 18:43:49 UTC

Description Paulo Andrade 2015-02-04 22:13:50 UTC
Created attachment 988311 [details]
test3.ksh

The problem with a possible patch has been report upstream at:
http://lists.research.att.com/pipermail/ast-users/2015q1/004723.html

On the sample test case, every iteration of the loop
leaks some memory in src/cmd/ksh93/sh/array.c:

---8<---
/*
 *  This is the default implementation for associative arrays
 */
void *nv_associative(register Namval_t *np,const char *sp,int mode)
{
	register struct assoc_array *ap = (struct assoc_array*)nv_arrayptr(np);
	register int type;
	switch(mode)
	{
	    case NV_AINIT:
		if(ap = (struct assoc_array*)calloc(1,sizeof(struct assoc_array)))
---8<---

that calloc is leaking memory. After a few tests, I noticed
that if using "unset -f" in the test case, the leak would
no longer happen. But that is not supposed to be the way
to fix it, nor leaks are supposed to happen if not using it.

The proposed fix was done observing that in
src/cmd/ksh93/bltins/typeset.c, when using the -f option,
it would have this code:

...
		case 'f':
			troot = sh_subfuntree(1);
...
otherwise, it would have shp->var_tree passed as argument,
or set again to it if using the 'n' or 'v' unset options.

But if troot==shp->var_tree, it would continue in either
of these:

---8<---
#if SHOPT_FIXEDARRAY
				if((ap=nv_arrayptr(np)) && !ap->fixed  && name[strlen(name)-1]==']' && !nv_getsub(np))
#else
				if(nv_isarray(np) && name[strlen(name)-1]==']' && !nv_getsub(np))
#endif /* SHOPT_FIXEDARRAY */
---8<---

The email to upstream asks for the reason of the continue,
as it is causing a leak, so whatever is the reason, it is
buggy, and no test case exercises whatever it prevents, as
with or without the proposed patch, %check results are
basically the same.

Comment 1 Paulo Andrade 2015-02-04 22:15:07 UTC
Created attachment 988312 [details]
ksh-20120801-assoc-unset-leak.patch

This patch corrects the problem.

Comment 3 Paulo Andrade 2015-02-05 00:29:29 UTC
Created attachment 988343 [details]
test3.ksh

Previous reproducer was using "unset -f" (from a debug test),
what hides the problems.

Comment 4 Paulo Andrade 2015-02-09 11:35:27 UTC
Created attachment 989652 [details]
ksh-20120801-assoc-unset-leak.patch

This is I believe the best approach to correct the
problem. And from the alternatives I tried, the one
with less test regressions. But it is a starting
point, due to these failures in %check, that I
believe are another bug:

---8<---
arith.sh[708]: typeset: cannot change associative array A to index array

    arrays.sh[84]: number of elements of y is not 6
    arrays.sh[87]: string length of unset element is not 0
    arrays.sh[106]: number of elements of left in variable foo is not 0
    arrays.sh[114]: number of elements of left in variable foo again is not 0
    arrays.sh[395]: ${!var[sub]} should be var[sub]
    arrays.sh[427]: subscript not evaluated for unset variable
arrays.sh[430]: typeset: cannot change associative array foo to index array

    bracket.sh[309]: ${foo[1]+x} should be empty
    bracket.sh[311]: bar[1] should not be set
    bracket.sh[315]: z[1] should not be set
    bracket.sh[320]: y[1] should not be set
---8<---

If I understand correctly, the above are mostly a side
effect of previously something like:

unset foo[bar]

straight releasing toplevel "foo"  (and leaking)
instead of releasing "foo[bar]".

Comment 5 Paulo Andrade 2015-02-10 20:51:50 UTC
Created attachment 990258 [details]
ksh-20120801-assoc-unset-leak.patch

This patch has no regressions in the ksh test cases,
but may still be incomplete for special cases. It
releases memory just before it would become unreachable.

Posted upstream at
http://lists.research.att.com/pipermail/ast-users/2015q1/004729.html

Comment 6 Paulo Andrade 2015-02-12 13:42:48 UTC
For the record, David Korn told me in a private email there
could be issues with indexed arrays. Basically, using
"typeset -a" and numeric indexes, instead of "typeset -A"
and symbolic indexes in the test case. I tested it and
indexed arrays do not leak, neither need the patch.
So, I do not know of other special cases that could
leak, and need patch customizations.

Comment 16 errata-xmlrpc 2015-07-22 06:56:34 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2015-1450.html


Note You need to log in before you can comment on or make changes to this bug.