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

Why does flwr override the signal handlers in flwr.client.app? #3837

Open
scarere opened this issue Jul 17, 2024 · 2 comments
Open

Why does flwr override the signal handlers in flwr.client.app? #3837

scarere opened this issue Jul 17, 2024 · 2 comments
Labels
question Further information is requested

Comments

@scarere
Copy link

scarere commented Jul 17, 2024

What is your question?

I noticed flwr 1.9.0 overrides the SIGINT and SIGTERM signal handlers in flwr.client.app (this was not present in 1.7.0). The custom defined signal handler just raises a StopIteration error every time. This was causing errors in my application where the client spawns some child processes for dataloading. When the client is shutdown, these child processes are terminated and the StopIteration error is raised. As I understand it, SIGTERM is the polite way to close a process, so I don't know why an error is needed. I've currently worked around this by temporarily resetting the signal handlers when the child processes are spawned but am curious to know if there is a better solution and what the reasoning behind this change was.

@scarere scarere added the question Further information is requested label Jul 17, 2024
@adam-narozniak
Copy link
Member

@charlesbvll @Robert-Steiner is this related to the stopping problems that you have been working on? Maybe you know more context for that?

@charlesbvll
Copy link
Member

@scarere Thank you for your feedback! This is very useful to us. We wanted to find a way that allows clients to exit gracefully by handling signals. We didn't realise that this approach could have bad consequences for child processes. We will definitely take this into account in our implementation. I think @panh99 is already working on a better approach for handling signals, which will hopefully fix this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants