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.

Sign up for GitHub

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

Jump to bottom

avoid race in emitting event by using loop.call_soon_threadsafe() #119

Merged
merged 1 commit into from
Jul 23, 2018
Merged

avoid race in emitting event by using loop.call_soon_threadsafe() #119

merged 1 commit into from
Jul 23, 2018

Conversation

wjwwood
Copy link
Member

@wjwwood wjwwood commented Jul 19, 2018

This could result in an error like this:

[DEBUG] [launch]: emitting event synchronously: 'launch.events.IncludeLaunchDescription'
[DEBUG] [launch]: emitting event synchronously: 'launch.events.IncludeLaunchDescription'
[DEBUG] [launch.LaunchService]: processing event: '<launch.events.include_launch_description.IncludeLaunchDescription object at 0x101fadd30>'
[DEBUG] [launch.LaunchService]: processing event: '<launch.events.include_launch_description.IncludeLaunchDescription object at 0x101fadd30>' ✓ '<launch.event_handlers.on_include_launch_description.OnIncludeLaunchDescription object at 0x101fad978>'
[DEBUG] [launch.LaunchService]: processing event: '<launch.events.include_launch_description.IncludeLaunchDescription object at 0x101fadd68>'
[DEBUG] [launch.LaunchService]: processing event: '<launch.events.include_launch_description.IncludeLaunchDescription object at 0x101fadd68>' ✓ '<launch.event_handlers.on_include_launch_description.OnIncludeLaunchDescription object at 0x101fad978>'
[INFO] [launch]: process[lifecycle_talker-1]: started with pid [69139]
[INFO] [asyncio]: %s: %r
[DEBUG] [launch]: emitting event: 'launch.events.process.ProcessStarted'
[DEBUG] [launch.LaunchService]: processing event: '<launch.events.process.process_started.ProcessStarted object at 0x1021a6588>'
[DEBUG] [launch.LaunchService]: processing event: '<launch.events.process.process_started.ProcessStarted object at 0x1021a6588>' ✓ '<launch.event_handler.EventHandler object at 0x101fadb38>'
[DEBUG] [launch]: emitting event synchronously: 'launch_ros.events.lifecycle.StateTransition'
Exception in handling of 'lifecycle.msg.TransitionEvent': Non-thread-safe operation invoked on an event loop other than the current one
[DEBUG] [launch]: emitting event synchronously: 'launch_ros.events.lifecycle.StateTransition'
^C[WARNING] [launch.LaunchService]: user interrupted with ctrl-c (SIGINT)

If a data race failed, which was more common if the lifecycle node (in this case talker from the package lifecycle) did not print anything to stdout.

@wjwwood wjwwood added bug Something isn't working in review Waiting for review (Kanban column) labels Jul 19, 2018
@wjwwood
Copy link
Member Author

wjwwood commented Jul 19, 2018 •
edited
Loading

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

sloretz
sloretz approved these changes Jul 23, 2018
View reviewed changes
Copy link
Contributor

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

LGTM,

Learned about asyncio.Queue too.

@wjwwood
Copy link
Member Author

wjwwood commented Jul 23, 2018

Thanks @sloretz 🍭.

@wjwwood wjwwood merged commit b608581 into master Jul 23, 2018
@wjwwood wjwwood deleted the fix_lifecycle_event_race branch July 23, 2018 18:44
@wjwwood wjwwood removed the in review Waiting for review (Kanban column) label Jul 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sloretz sloretz sloretz approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@wjwwood @sloretz