ConnectionManager can now be destroyed safely

[allada]

Reduces chance of any issues related to ConnectionManager not being destroyed safely due to un-terminated spawns.

---

This change is 

[allada]

cc: @chrisstaite-menlo , @zbirenbaum

+@adam-singer

Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Local / ubuntu-22.04, Publish image, Publish nativelink-worker-lre-cc, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, vale, windows-2022 / stable, zig-cc ubuntu-20.04, zig-cc ubuntu-22.04 (waiting on @adam-singer)

[adam-singer]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: 1 of 1 LGTMs obtained, and pending CI: Remote / large-ubuntu-22.04

[chrisstaite]

I don't think this is the best solution. I think waiting for EOF on both RX in the worker makes more sense. The ownership of the worker is by itself and making the ConnectionManager block on Drop seems an unnecessary hack.

[chrisstaite]

nativelink-util/src/connection_manager.rs line 225 at r1 (raw file):

                        // The ConnectionManager was dropped, shut down the
                        // worker.
                        break;

We should add a loop here waiting for connection_rx to return None and then we'll be done. None of the other changes are necessary.

[allada]

This solution did not work as I expected anyway...

The long vision is to be able to have Nativelink be able to be given a new config at runtime, and it'd construct an entirely different set of stores/schedulers/exc... (or recycle them) and then start dispatching work to the new services on request receives without needing to restart Nativelink.

In other words, change configs without any down time.

To do this we need to ensure all our structs can be destroyed safely. I'll look deeper, but in reality, I'd prefer if there was a way to confirm that a component and all allocated resources are freed, but it's tricky due to rust's way of dealing with Drop in a synchronous way.

Reviewable status: 1 of 1 LGTMs obtained, and 1 discussions need to be resolved