-
Notifications
You must be signed in to change notification settings - Fork 1
Open
Labels
Description
This is in ThunkIoDisk.spawn_in_directory: change directory, run command, change directory back. But changing the process working directory implicitly makes only it possible to only run one form function at a time.
And since the spawn is not protected by a mutex, where everything in dk build / mlfront-shell is cooperatively threaded, we'll get difficult to understand bugs because of race conditions.
Ideas:
- Use
cmake -E chdir <program> <args>...style spawning of commands. To avoid distributing extra binaries, usemlfront-shell -E chdir <program> <arg>...where-Ewill short-circuit the CLI argument/option processing and can be uniform betweenmlfront-shellanddk. Problems: As a reference implementation the reliance on argv[0] having a-E chdirmode is hard to translate into other languages. - Use https://ocaml.org/p/spawn/latest/doc/spawn/Spawn/index.html, but be prepared to vendor it because Jane Street likes bumping up their OCaml minimum versions once across all projects
- Wrap https://github.com/DaanDeMeyer/reproc
- (move out) For supervisord style of spawning, use Apache commons daemon native binaries. The binaries will need to be embedded like was done for GitHubCLI attestations. This can be peeled off into its own issue since options will need to be added to JSON (a
service: { start: {args:...}, stop: {args:...} }alongside a modifiedfunction_: {services_required:[...]}), a new subcommandstart-service, and logic added to garbage collect the service when either the user or set of functions is finished with the service. Make its API a small subset of systemd so that on Linux systemd can be used directly instead of a commons daemon wrapper.
Also, for safety, we should shadow the Stdlib and remove the change directory function so that this easy-to-miss type of bug is much less possible.