Add SIGINT signal handlers for Windows #3167
Conversation
Ace314159
force-pushed
the
gazebo11
branch
2 times, most recently
from
bec0e75
to
f350aca
Compare
|
Is there anything else I should do to get this PR merged? |
|
@j-rivero I'd really appreciate a review, thank you! |
|
Hey @Ace314159, sorry for the radio silence. I was looking into yes, code changes looks good to me but I don't have a working Windows environment these days. I'm going to approve but let's wait until someone can verify that indeed improves the runtime shutdown. Thanks for the contribution! |
|
I restarted the CI to check if the failure were flaky. |
|
Do you think the failures are caused by my changes? I'm having trouble actually looking at the failing tests. Most of them just say that the test didn't run. |
|
Can you rebase to the latest gazebo11 dev branch? |
|
@talregev I've rebased |
|
@traversaro Can you review it? |
Sure, as @j-rivero mentioned it would be nice to verify that this indeed improve the ctrl+c behavior on Windows. @talregev did you tested this improvements yourself? If yes, I think we can proceed. |
|
@Ace314159 Can you record your screen while you press ctrl+c and post it here? I will also try to do it when I have time and post it here. |
|
@traversaro I build gazebo with vcpkg for x86 on this branch, and I can confirm that when I press crtl+c, it close all windows immediately. @traversaro Let me know what do you think. 2023-10-10.09.11.33.mp4 |
|
Perfect, thanks! I will wait two days for anyone else to comment, then I will merge. Feel free to ping then, thanks! |
|
@traversaro gentle ping about this PR. |
|
Thanks! |
This makes ctrl+c more reliable at actually killing the process.