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 closing behavior when attempting to close a device implementing the service interface #3100

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

S-Dafarra
Copy link
Contributor

I am implementing a device which can close prematurely depending on the internal state. I want yarpdev to be notified when this happens and close consequently.

In order to have this behavior, I implemented the IService interface. But then, if I press CTRL+C in the yarpdev terminal, it seems to always get stuck, and then I have to kill the yarpdev process. The last message was

[INFO] |yarp.dev.Drivers| [try 1 of 3] Trying to shut down /yarpdev/quit

After some debugging, I noticed that when the signal handler was setting the static flag "terminated" to true, it was also attempting to close immediately the terminator port. But the terminator port is receiving the termination message and some race condition seemed to occur on Windows.

If we avoid to set the terminated flag from the signal handler and instead we wait for the termination port to do its job, then yarpdev closes smoothly without issues.

cc @randaz81 @Nicogene @traversaro

…the service interface

Apparently, when the signal handler was setting the static flag "terminated" to false, it was also attempting
to close immediately the terminator port, but this was receiving the termination message and some race condition
seemed to occur on Windows
@S-Dafarra S-Dafarra requested a review from randaz81 as a code owner March 29, 2024 16:27
Copy link

update-docs bot commented Mar 29, 2024

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would update the release notes by adding a file in doc/release/<target_branch>, based on your changes.

Copy link

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.

1 participant