search

slp / krun

krun - run programs from your system in a microVM
MIT License
25 stars 7 forks source link

krun-server should outlive first command #33

Closed teohhanhui closed 3 weeks ago

teohhanhui commented 4 weeks ago

(Continuing from https://github.com/slp/krun/pull/23#discussion_r1624008269)

With some real-world testing, it's entirely jarring behaviour from a user perspective.

Steps for reproduction:

  1. Launch krun FEXBash
  2. Launch krun FEXInterpreter steam
  3. exit from FEXBash

Expected result: Steam should continue running.

Actual / observed behaviour: Steam is killed abruptly.

slp commented 4 weeks ago

Something we can do, in krun-server, is keep track of the children and, if there's one that's still alive when the main command exits, inform the user and keep waiting until Ctrl+C is hit, or there are no more children running.

This behavior would also work well with binfmt integration. First command run, even from a Desktop link, will start up the VM, which will be kept alive until the last command exits.

WDYT?

teohhanhui commented 4 weeks ago

Sounds reasonable. I'll send a PR.

teohhanhui commented 3 weeks ago

Did some digging: https://users.rust-lang.org/t/how-to-properly-close-a-tcplistener-in-multi-thread-server/87376

Track requests, not threads.

https://users.rust-lang.org/t/how-to-properly-close-a-tcplistener-in-multi-thread-server/87376/17

shutdown does not affect accept, only recv... But OTOH, we don't need to care about the server thread being blocked on accept, as long as all in-flight requests are handled (and also wait for child processes to exit)... So, we in fact do NOT want to join the server thread.

We also don't want to use Drop as it's not unwind safe:

teohhanhui commented 3 weeks ago

Okay, I think we really need to rethink our architecture here. This is not going to work in any user friendly way.

Waiting for children to exit still means new krun invocations in the meantime would fail for no apparent reason. That's very bad UX.

It seems to me the only possible solution without involving a long-lived krun server running in the background would be: delay the shutdown if a new incoming connection is accepted. (If we do anything to stop listening, we're back to the above problem with new krun invocations failing...)

I'll continue thinking and working on this.