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

Fix hang on keyboard interrupt #69

Merged
merged 1 commit into from Feb 20, 2018

Conversation

Projects
None yet
5 participants
@rhololkeolke
Copy link
Contributor

rhololkeolke commented Jan 21, 2018

Relates to #64

When keyboard interrupt exception occurs loop.run_forever is
called, but there is no loop.stop call. This causes a hang.

I don't have a windows machine to test on.

Fix hang on keyboard interrupt
Fixes #64

When keyboard interrupt exception occurs loop.run_forever is
called. But there is no loop.stop call. This causes a hang.

@mikaelarguedas mikaelarguedas requested a review from dhood Feb 1, 2018

@Karsten1987

This comment has been minimized.

Copy link
Contributor

Karsten1987 commented Feb 7, 2018

I can confirm that this fixes the problem on my mac machine. Will try to run CI on this asap.

@mikaelarguedas mikaelarguedas requested a review from Karsten1987 Feb 8, 2018

@Karsten1987

This comment has been minimized.

Copy link
Contributor

Karsten1987 commented Feb 9, 2018

Unfortunately, I believe this doesn't fix the issue on Windows.

@mikaelarguedas

This comment has been minimized.

Copy link
Contributor

mikaelarguedas commented Feb 15, 2018

Thanks @rhololkeolke for the contribution.
As this fixes the issue on some of the platforms we will take this as an incremental improvement. PR description has been modified to not close the original issue as this patch is not sufficient to fix it.
@Karsten1987 or @dhood to run CI and merge if green

@Karsten1987

This comment has been minimized.

Copy link
Contributor

Karsten1987 commented Feb 17, 2018

CI:

Build Status
Build Status
Build Status

@dhood

dhood approved these changes Feb 20, 2018

@mikaelarguedas

This comment has been minimized.

Copy link
Contributor

mikaelarguedas commented Feb 20, 2018

Test failures on Windows and MacOS unrelated to this PR: ros2/build_cop#83.
Merging...

@mikaelarguedas

This comment has been minimized.

Copy link
Contributor

mikaelarguedas commented Feb 20, 2018

Thanks @rhololkeolke for the patch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.