Re: Bug#953421: dash: Resident Set Size growth is unbound (memory leak) on an infinite shell loop

[Jilles Tjoelker <jilles@xxxxxxxx>]

On Sun, Mar 29, 2020 at 08:06:31PM +0100, Harald van Dijk wrote:
> On 29/03/2020 18:54, Vitaly Zuevsky wrote:
> > I have now fixed this bug locally.

> > The leak is in jobtab array (jobs.c). I concluded that the most
> > logical approach would be eliminating inconsistency between
> > makejob() and dowait() functions. My fix in a forked repo:

> > https://salsa.debian.org/psvz-guest/dash/-/commit/5e3ea90cb3355d1308c482661a471883d36af5e7

> This change is incorrect. The reason dash keeps on allocating memory is
> because dash needs to keep on allocating memory. Consider this script:

>   set -- $(seq 1 100)
>   for i
>   do
>     : &
>     sleep .1
>   done
>   for i
>   do
>     wait %$i
>   done

> This is a valid script and works fine in dash. Your change breaks this by
> not keeping the jobs around long enough, and I hope this test script shows
> that there is no way to keep the jobs around long enough but by allocating
> ever more memory.

I agree that the change is incorrect, but I do not agree that this kind
of script must leak memory. Per POSIX.1-2008 XCU 2.9.3.1 Asynchronous
Lists, an implementation has additional ways to forget about jobs than
just an appropriate completion of the wait utility: if another
asynchronous job is started when $! was not referenced or if the number
of known process IDs would exceed {CHILD_MAX} (which tends to be rather
big, though).

POSIX does not seem to expect using %<jobid> in scripts like this; it
seems highly fragile to me anyway (although $! has problems with process
ID reuse).

FreeBSD sh implements forgetting when $! was not referenced (and the job
has terminated), but not the {CHILD_MAX} limit. This avoids the
increasing memory usage in the example script.

> Your change makes it impossible to keep track of the background process's
> status, but if you do not care about that anyway, you can avoid the
> increasing memory use without modifying dash by launching a background
> process without including it in the current shell's job table, by launching
> it from a subshell:

>   while true
>   do
>     (true &)
>     sleep .1
>   done

Certainly a good idea.

Another option may be to include regular invocations of the wait utility
without parameters, although this is not suitable for all scripts.

-- 
Jilles Tjoelker