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

Hotfix for PX4 application not exiting anymore on Cygwin #11305

Merged
merged 1 commit into from
Jan 28, 2019

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Jan 26, 2019

Hotfix for #11027

Test data / coverage
SITL on Cygwin (Wndows)

Describe problem solved by the proposed pull request
Some threads do not exit and are still running when trying to exit SITL running under Windows in Cygwin.

This problem was introduced with:

Describe your solution
I leave the SIGINT handler on its default implementation for Cygwin which kills the process and all its threads when pressing Ctrl+C.

This hotfix at least allows the usage of Ctrl+C again instead of forcing the user to use the task manager to shut down px4.exe going crazy on CPU load instead of exiting everytime.

Describe possible alternatives
An analysis of what exactly goes wrong when trying to exit gracefully would be the desired way but my limited experience with debugging the overly complex thread structure of PX4 through multiple wrapping layers and my already spent countless hours on this problem lead me to this hotfix.

@MaEtUgR MaEtUgR added the bug label Jan 26, 2019
@MaEtUgR MaEtUgR self-assigned this Jan 26, 2019
@MaEtUgR MaEtUgR requested review from bkueng and julianoes January 26, 2019 13:09
@MaEtUgR MaEtUgR force-pushed the windows-exiting-hotfix branch from b6bf04e to 26d2496 Compare January 26, 2019 13:11
@dagar
Copy link
Member

dagar commented Jan 26, 2019

This seems wrong, but understandable.
How about at least adding a detailed explanation and a TODO in the signal handler?

@LorenzMeier
Copy link
Member

Todo and link to the issue would be good.

@LorenzMeier
Copy link
Member

I meant in the code.

@MaEtUgR MaEtUgR force-pushed the windows-exiting-hotfix branch from 26d2496 to 5770a5d Compare January 27, 2019 07:12
@MaEtUgR
Copy link
Member Author

MaEtUgR commented Jan 27, 2019

I adjusted the comment to include a todo and a link.

Some threads do not exit and are still running when
trying to exit SITL running under Windows in Cygwin.

This problem was introduced with:
- posix shell #10173 because of strating a child process
for the startup script and mixing up the signal handling
(only Ctrl+C broken)
- lockstep #10648 because of simulator threads not
stopping gracefully anymore with timing race conditions
(no graceful exit possible anymore)

I leave the SIGINT handler on its default implementation for
Cygwin which kills the process and all its threads when pressing
Ctrl+C.

This hotfix at least allows the usage of Ctrl+C again instead
of forcing the user to use the task manager to shut down
px4.exe going crazy on CPU load instead of exiting
everytime.
@bkueng bkueng merged commit bda60ec into master Jan 28, 2019
@bkueng bkueng deleted the windows-exiting-hotfix branch January 28, 2019 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants