converged sidecar fixes

[colinmcardell]

sidecar: client concurrency, transport failures reported to lua, and the last in-process forks removed

why

the sidecar exists so the converged norns binary never fork()s while holding the JACK real-time thread. some paths still forked in-process, or shelled out for data the process can read itself. also, the sidecar client had a couple of threading bugs.

what

this PR brings in changes related to ongoing work on norns binary <-> sidecar communication.

this PR does not fix SYSTEM > UPDATE just yet. a follow-up PR will include sidecar/norns restructuring to resolve the update.sh regression and the unreliable restart issues in the converged norns binary.

• fix: the request queue was single-producer (BlockingReaderWriterQueue) with two producers, system_cmd from the matron lua thread and sidecar_client_cleanup's REQUEST_QUIT from the main thread. now BlockingConcurrentQueue. run_cmd_lock was never initialized, working only by static zero-init.
• fix: a regression in when system command outputs nothing. the lua callback never ran and its registry ref leaked. failures now carry an empty string so the callback always fires, restoring pre-converged scripting contract, etc.
• fix: stat shelled out to df, vcgencmd, and cat /proc/stat every 2 seconds. now statvfs(), /sys/class/thermal/thermal_zone0/temp, and /proc/stat read directly, dropping the vcgencmd dependency.
• fix: the old cpu loop stopped after the aggregate line and cores 0 through 2, so norns.cpu[4] was never written and the home screen's fourth cpu figure has been reading a constant 0.. it now reports core 3.
• fix: settings password change, the tape dir, and expand_filesystem were the last in-process os.execute sites, now norns.system_cmd / util.make_dir.

verified

• tests passing.
• tested on hardware: password change, tape menu, stat readouts match df / vcgencmd, norns.cpu[4] goes live under load. stats work correctly on norns and norns shield.

note

the settings password change path historically has passed a plaintext password through to logs upon error. this PR does not fix this. i've intentionally left that alone for the moment.

[tehn]

fantastic! taking a look now.

@ngwese may be interested as he authored the original

[Dewb]

What are the implications for user scripts that use os.execute? Do we need to prevent that to keep from forking in the main process?

[tehn]

everything looks good to me. lots of little edges i didn't expect to be impacted--- really opened my eyes to how norns grew in complexity over the years

thank you!

[tehn]

What are the implications for user scripts that use os.execute? Do we need to prevent that to keep from forking in the main process?

i'm wondering if this will represent a breaking change we'll need to live with? a blocking os.execute will not play well with jack.

---

which brings me to the SYSTEM UPDATE issue that you mention, basically update.lua needs to be reworked for async system commands, correct? i see some issues there currently.

also, os.capture looks like it'll need to be deprecated or reworked.

[colinmcardell]

What are the implications for user scripts that use os.execute? Do we need to prevent that to keep from forking in the main process?

Yes. This question has been running through my head... I haven't tried to tackle the answer to that question at this point, but my initial thought has been an override to os.execute that runs it through sidecar. Not clear on the feasibility of this idea.

i'm wondering if this will represent a breaking change we'll need to live with? a blocking os.execute will not play well with jack.

That is one possibility also.

which brings me to the SYSTEM UPDATE issue that you mention, basically update.lua needs to be reworked for async system commands, correct? i see some issues there currently.

That's right. I have that work mostly done. It reworks update.lua <-> sidecar <-> update.sh, with a "detached" system command, async system command support, and an update process that swaps the binary once validated as to not brick. It's a lot of change in all three locations, but is working well, covered by unit tests, and I've e2e tested. This will be the impending follow up PR.

also, os.capture looks like it'll need to be deprecated or reworked.

The Lua util util.os_capture already routes through sidecar, so that's good!

[colinmcardell]

everything looks good to me. lots of little edges i didn't expect to be impacted--- really opened my eyes to how norns grew in complexity over the years

thank you!

no problem, my pleasure. I'm excited to get this work and the follow up work in. I also didn't expect these little edges along the way 😄

[tehn]

Yes. This question has been running through my head... I haven't tried to tackle the answer to that question at this point, but my initial thought has been an override to os.execute that runs it through sidecar. Not clear on the feasibility of this idea.

while this will work, a great many scripts expect the command to be blocking, so changing that would break functionality.

i'm curious what happens when the main thread gets blocked--- a slew of jack xruns? or does jack fail badly and quit?

if it's the first i suspect we'll be ok. scripts running os.execute during "performance" is hopefully unlikely--- though i know many use it for setup.

but also i think the converged update is such an improvement that it's worthwhile to suffer the breakage. i think people will help migrate/fix problem scripts. but the main problem i see is the potential quantity of abandoned scripts at this point, where the author may be difficult to get ahold of. (this is a separate conversation, sorry!)

[Dewb]

_norns.system_cmd is asynchronous but _norns.execute looks like it effectively has synchronous semantics even though it routes through the sidecar, is that right? If so we could just override os.execute with an implementation based on _norns.execute, just like util.os_capture.

[colinmcardell]

Yes, _norns.execute is blocking.

[colinmcardell]

i'm curious what happens when the main thread gets blocked--- a slew of jack xruns? or does jack fail badly and quit?

• _norns.execute blocks the matron main thread which means, screen, keys, encoders, clocks, MIDI
• os.execute style fork() from the norns binary will mean potential xrun yes, looking into if jack will potentially explode.

[colinmcardell]

https://github.com/monome/norns-image/blob/main/config/norns-jack.service#L11
-s will make it not explode. 😄

[colinmcardell]

update just to be clear here... I was half asleep on that last post.
the existing -s on the norns jack service configuration means that the jack service will not fail badly and quit, or more specifically, jack will not evict the norns jack client for failing to produced samples.

If so we could just override os.execute with an implementation based on _norns.execute, just like util.os_capture.

yes! this is a good experiment to try. I can look into it as part of the sidecar/update reworking.

[tehn]

That's good news and feels like we're perhaps in a more healthy position not to mandate breaking change.

[colinmcardell]

Good news. I've got an override within weaver of os.execute routed to sidecar tested and working. It's an addition under weaver I'm calling lua_shell that intercepts the Lua VM on init and reroutes to custom functions. io.popen received similar treatment.

I've had to make some further adjustments to sidecar but it's looking good. I'm going to cap the rework there and PR this weekend.

[tehn]

that's fantastic news! happy to test as always when you're ready.

[ngwese]

also happy to test this work on slightly non-standard hardware (factory norns with cm4s) which might be a good thing!

[colinmcardell]

I'm a little delayed from my expected timing. Nature and flooding decided I should focus on my basement. :|

[tehn]

nature does tend to make the decisions, i hope the situation improved and you get some relief from worrying about it (coming from someone very familiar with these situations!)