Fixing awful.spawn read_lines closure leak

[soderstrom-rikard]

Affects: awesome v4.3 and likely all prior versions
Tested on: awesome 4.3-6, lua-lgi 0.9.2-14, Lua 5.3.6

PROBLEM

awful.spawn.read_lines() has a memory leak caused by a mutual closure reference
cycle between start_read and finish_read that survives multiple Lua GC cycles.

OBSERVED SYMPTOM
awesome memory grows from ~400 MB at startup to 4+ GB over 1-2 days when any
widget using awful.widget.watch (or anything that calls easy_async on a timer)
is in use.

Forcing two GC cycles from awesome-client confirms the leak is entirely in
Lua-tracked objects, not native memory:

awesome-client 'return string.format("%.1f MB", collectgarbage("count")/1024)'
-- returns e.g. "2429.8 MB"

awesome-client 'collectgarbage("collect"); collectgarbage("collect"); \
  return string.format("%.1f MB", collectgarbage("count")/1024)'
-- returns e.g. "336.3 MB"  (2.1 GB freed)

ROOT CAUSE

In spawn.read_lines(), start_read and finish_read are declared as upvalues and
then close over each other:

local start_read, finish_read
start_read = function()
    stream:read_line_async(GLib.PRIORITY_DEFAULT, nil, finish_read)
end
finish_read = function(obj, res)
    ...
    start_read()   -- closes over start_read
end

This creates a mutual reference cycle:
finish_read <--> start_read (each is an upvalue of the other)

Additionally, LGI's ffi closure for finish_read (created by read_line_async)
holds a Lua registry reference (target_ref) to finish_read. This reference is
released only after:

Step 1: LGI's autodestroy guard is finalized by Lua GC
(guard was created in closure_callback when the callback fired)
Step 2: Lua GC detects finish_read is now unreachable

Because GI_SCOPE_TYPE_ASYNC callbacks always use LGI's autodestroy guard
mechanism (see lgi/marshal.c:marshal_2c_callable), there is an unavoidable
one-cycle delay between the callback firing and the Lua reference being
released. After that, the mutual cycle between start_read and finish_read
requires ANOTHER cycle to detect as unreachable.

In practice this means the closures, their upvalues (stream, line_callback,
done_callback, accumulated stdout strings etc.) survive 3+ GC cycles per
easy_async call. With awful.widget.watch running at 1 Hz (volume widget
default), this accumulates ~50-200 KB of Lua objects per second faster than
Lua's incremental GC (with default gcpause=200) reclaims them, resulting in
~180 MB per hour or 4.32 GB per day of retained memory.

The ffi closures themselves (via ffi_closure_alloc) are NOT tracked by Lua's
allocator, so growing ffi memory does not increase GC pressure, making the
problem worse.

FIX

Break all upvalue references inside done() before calling done_callback. This
eliminates both the start_read/finish_read mutual cycle and all other upvalue
chains (stream, line_callback, done_callback) in a single step. After this:

• LGI's guard finalizes -> releases target_ref -> finish_read unreachable
• finish_read is collected in the very next GC cycle (no cycle detection
  needed since the mutual reference was already broken)
• stream, line_callback, done_callback follow in the same sweep

This reduces from 3+ GC cycles to 2, and eliminates the mutual-cycle detection
overhead entirely.

SUGGESTED GC TUNING (companion change, rc.lua or early in awesome config)

Even with this patch, Lua's default GC settings (gcpause=200, meaning GC starts
a new cycle when memory reaches 200% of the post-collection size) allow large
amounts of short-lived objects to accumulate before collection. For a desktop
WM with frequent async spawns, more aggressive settings are advisable:

-- In rc.lua, near the top:
collectgarbage("setpause",   110)  -- start new cycle at 110% (near-continuous)
collectgarbage("setstepmul", 400)  -- GC does 4x work per allocation step

This is independent of the spawn.lua fix and provides a safety margin for any
other sources of short-lived object accumulation.

[actionless]

thanks for investigating!

could you provide more info on your debugging setup and whatever it could be somehow automated for potentially detecting similar problems, as i think there must few more callbacks in the lua code

[Aire-One]

@actionless do you know why the CI didn't start? I can only see the Mergify in the checks tab...

Even with this patch, Lua's default GC settings (gcpause=200, meaning GC starts
a new cycle when memory reaches 200% of the post-collection size) allow large
amounts of short-lived objects to accumulate before collection. For a desktop
WM with frequent async spawns, more aggressive settings are advisable:

hmm... I'm not sure about making this the default. More regular GC work also means the main thread got busy/frozen more often, which is not something you want for a WM. We would need to study the memory space saved VS runtime latency introduced by such parameters. I guess benchmark results would greatly depend on the hardware used, and it's probably fine with nowadays computers anyway 🤷