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

simulator: clean shutdown of socket #1345

Merged
merged 1 commit into from
Dec 30, 2024

Conversation

NickeZ
Copy link
Collaborator

@NickeZ NickeZ commented Dec 19, 2024

If we don't close the socket we have to wait a couple of minutes until the OS cleans up after us. This is quite annoying because then you can't launch the simulator with the same port immediately after you kill it.

@NickeZ NickeZ requested review from benma and asi345 December 19, 2024 10:01
@NickeZ NickeZ force-pushed the nickez/simulator-clean-shutdown branch from f5e4a1c to 0d8b5de Compare December 19, 2024 10:02
Copy link
Contributor

@asi345 asi345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it. When the python client first quits and then simulator is given CTRL-C, it works really well. I can instantly restart a new communication at the same port.

However, when the simulator first gets CTRL-C without quitting from python client, simulator gets stuck and can not quit until python client quits.

If this is expected and okay for you, I can approve.

Copy link
Collaborator

@benma benma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@NickeZ
Copy link
Collaborator Author

NickeZ commented Dec 30, 2024

I made some attempts at a clean shutdown when a client still is connected. I think the client has to do a proper shutdown when it detects that the server is down for the OS to release the port, so we might have to do some changes to the client as well. Anyway, it seems complicated enough that I can do that in a follow up PR. This PR still improves the situation when no clients are connected.

@NickeZ NickeZ merged commit c93367d into BitBoxSwiss:master Dec 30, 2024
3 checks passed
@Noxet
Copy link

Noxet commented Dec 30, 2024

You should avoid using non-reentrant functions, like printf, in signal handlers.
The only functions that POSIX guarantee to be async-signal-safe, e.g. write(), are listed here

@NickeZ
Copy link
Collaborator Author

NickeZ commented Dec 30, 2024

You should avoid using non-reentrant functions, like printf, in signal handlers. The only functions that POSIX guarantee to be async-signal-safe, e.g. write(), are listed here

addressed in #1353, thanks for the feedback!

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.

4 participants