Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Clone, Copy for NestedSubsystem #63

Closed

Conversation

9999years
Copy link

Hello! Thanks for this library! I've added implementations of Clone and Copy for NestedSubsystem. This makes it easier to perform a partial shutdown.

@Finomnis
Copy link
Owner

Finomnis commented Sep 24, 2023

Having NestedSubsystem not copy or clone is on purpose, so that perform_partial_shutdown cannot be called multiple times. Otherwise there might be weird race conditions.

If you have a better idea on how to deal with repeated requests to parial shutdowns of the same subsystem, feel free to suggest them.

The base problem here is that the internal implementation of this crate is questionable in hindsight. Especially everything surrounding partial shutdowns is kind of a dirty hack. The idea of having errors propagate to the top, but then dynamically reroute them to the partial shutdown call if one exists is kind of a really bad idea in hindsight that caused a lot of headaches in maintaining this crate.

I'm in the process of a complete rewrite which should simplify and improve things. Sadly, due to external complications that I have no control over, I currently have very little spare time. I'm trying to continue working on it whenever I can, but I cannot promise any deadlines right now.

While the problem this crate tries to solve sounds easy at first, it has a lot of hidden complications in the details. The two really hard topics here are error routing/handling and ownership (specifically preventing ownership cycles). I solved most of those problems in the rework already (theoretically, integration still pending), but it might still take some time.

@9999years 9999years closed this Sep 25, 2023
@9999years
Copy link
Author

Hmm, yes, I did think the partial shutdown mechanism was a little weird. I think in practice you'd usually want partial restarts rather than partial shutdowns on their own. (I'm reminded of the interesting system nested restarts that the Apollo Guidance Computer implemented that I learned about from this fascinating talk by Robert Wills.)

I also really want a multiple stage shutdown mechanism, at least for my use-case: I want to do a graceful shutdown first, then if the user presses Ctrl-C again I want to do a hard shutdown (SIGKILL to child processes). I'm not sure if that's something you've thought of.

I could probably devote some time to helping fix this library up if you'd like — let me know.

@Finomnis
Copy link
Owner

I could give you viewing rights for my rework repository, to get some guidance on API design. I'm not sure if it's worth putting much effort into the existing code.

Let me know if that's something you are interested in.

@9999years
Copy link
Author

Sure, I'm definitely curious to look at it.

@Finomnis
Copy link
Owner

Finomnis commented Sep 26, 2023

Do you have discord? If so, let's continue there. (Finomnis#5141, or simply finomnis)

@Finomnis Finomnis reopened this Sep 26, 2023
@Finomnis
Copy link
Owner

Accidentally pressed the reopen button on my phone.

@Finomnis Finomnis closed this Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants