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

improve ManagedProcess cleanup of resources #175

Open
pnwamk opened this issue Jul 16, 2021 · 2 comments
Open

improve ManagedProcess cleanup of resources #175

pnwamk opened this issue Jul 16, 2021 · 2 comments

Comments

@pnwamk
Copy link
Contributor

pnwamk commented Jul 16, 2021

The ManagedProcess class currently only relies on __del__ to clean up its underlying process, which can be unpredictable. Having such code as a failsafe may be reasonable, but we should add more explicit methods for cleaning things up in a predictable manner.

Thanks @kquick for pointing out.

@pnwamk
Copy link
Contributor Author

pnwamk commented Jul 16, 2021

Summary of initial suggestion from a quick peruse from Kevin:

For subclasses of ManagedProcess shutdown might need to be subclass-specific.

Perhaps add @abstractmethod to ServerProcess for the shutdown and go from there,
being aware Python's weak add-on typing won't lead you to all the places that might 
need to call that method.

@pnwamk
Copy link
Contributor Author

pnwamk commented Jul 16, 2021

N.B., this has the potential to affect the non-HTTP based connections. The HTTP connections rely almost entirely on the requests.post method for all comms handling.

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

No branches or pull requests

1 participant