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

MultiThreadedExecutor with wall timer hung #780

Closed
Mygao opened this issue Jul 9, 2019 · 15 comments
Closed

MultiThreadedExecutor with wall timer hung #780

Mygao opened this issue Jul 9, 2019 · 15 comments
Labels
bug Something isn't working

Comments

@Mygao
Copy link

Mygao commented Jul 9, 2019

Bug report

Required Info:

  • Operating System:
    • Ubuntu 18.04.2 LTS
  • Installation type:
    • binaries
  • Version or commit hash:
    • Dashing 0.7.6
  • DDS implementation:
    • Fast-RTPS
  • Client library (if applicable):
    • rclcpp

Steps to reproduce issue

This can be reproduced by modifying ros2 composition examples source code: SingleThreadedExecutor->MultiThreadedExecutor (https://github.com/ros2/examples/blob/master/rclcpp/minimal_composition/src/composed.cpp)


#include <memory>
#include "minimal_composition/publisher_node.hpp"
#include "minimal_composition/subscriber_node.hpp"
#include "rclcpp/rclcpp.hpp"

int main(int argc, char * argv[])
{
  rclcpp::init(argc, argv);
  rclcpp::executors::MultiThreadedExecutor exec;
  rclcpp::NodeOptions options;
  auto publisher_node = std::make_shared<PublisherNode>(options);
  auto subscriber_node = std::make_shared<SubscriberNode>(options);
  exec.add_node(publisher_node);
  exec.add_node(subscriber_node);
  exec.spin();
  rclcpp::shutdown();
  return 0;
}

Expected behavior

Message output goes on and on with count increasing without hanging

Actual behavior

Message output hangs. For example,
ros2 run examples_rclcpp_minimal_composition composition_composed
[INFO] [publisher_node]: Publisher: 'Hello, world! 0'
[INFO] [subscriber_node]: Subscriber: 'Hello, world! 0'
[INFO] [publisher_node]: Publisher: 'Hello, world! 1'
[INFO] [subscriber_node]: Subscriber: 'Hello, world! 1'

Stops here.

Additional information

I tested on 2 systems: Desktop and Laptop with both Ubuntu 18.04.2 LTS. Same ROS2 Dashing setups.


Feature request

Feature description

Implementation considerations

@gavanderhoorn
Copy link
Contributor

Context: [ros2] multi threaded executor with single node always makes the wall-timer hung. on ROS Answers.

Related (according to OP): #702.

@ivanpauno
Copy link
Member

I couldn't reproduce the problem in master.

  • Installation type:

    • binaries

Did you use the binaries of the first release, or Dashing release patch 1 binaries?

Steps to reproduce issue

This can be reproduced by modifying ros2 composition examples source code: SingleThreadedExecutor->MultiThreadedExecutor (https://github.com/ros2/examples/blob/master/rclcpp/minimal_composition/src/composed.cpp)

Did you only download this repo (examples) and build it in a overlay workspace of the binary installation? Please, share details of how you built it.

@ivanpauno ivanpauno added bug Something isn't working more-information-needed Further information is required labels Jul 9, 2019
@Mygao
Copy link
Author

Mygao commented Jul 10, 2019

The first release was installed with debian package installation (https://index.ros.org/doc/ros2/Installation/Dashing/Linux-Install-Debians/).
I cloned the examples repo and built it with colcon. I used the first release binaries of ROS2 as environment.
Now I tried with newer release of ROS2 Dashing with 'apt upgrade' command and same problem occurs.
I think I should try building ROS2 from source.

@Mygao
Copy link
Author

Mygao commented Jul 11, 2019

I tried with ROS2 built from source (latest-release) and the example above(running with MultiThreadedExecutor) worked fine.

@ivanpauno
Copy link
Member

Now I tried with newer release of ROS2 Dashing with 'apt upgrade' command and same problem occurs.

I will also try from binaries, to double check.

I tried with ROS2 built from source (latest-release) and the example above(running with MultiThreadedExecutor) worked fine.

Thanks for checking it! I will try to figure out what PR solved the problem, and add it to the patch release 2 list (if it's not already there).

@ivanpauno
Copy link
Member

I will also try from binaries, to double check.

I confirm that it isn't working when installing from debians. I have not clue about what PR solved the problem.

@ivanpauno ivanpauno removed the more-information-needed Further information is required label Jul 11, 2019
@ivanpauno ivanpauno added this to Proposed in Dashing Patch Release 2 via automation Jul 11, 2019
@nuclearsandwich
Copy link
Member

I tried with ROS2 built from source (latest-release) and the example above(running with MultiThreadedExecutor) worked fine.

Thanks for checking it! I will try to figure out what PR solved the problem, and add it to the patch release 2 list (if it's not already there).

@ivanpauno if they were building release-latest from source then it should be identical to the current dashing release and there wouldn't be any PRs that are not already part of the binaries.

@Mygao or @ivanpauno have either of you tried reproducing this issue with the Dashing binary archive? https://github.com/ros2/ros2/releases/tag/release-dashing-20190614

That might add an additional data point.

@ivanpauno
Copy link
Member

@ivanpauno if they were building release-latest from source then it should be identical to the current dashing release and there wouldn't be any PRs that are not already part of the binaries.

I didn't try with release-latest. I just checked with master (from source), and it was working.

@Mygao or @ivanpauno have either of you tried reproducing this issue with the Dashing binary archive? https://github.com/ros2/ros2/releases/tag/release-dashing-20190614

No, I only tried with debians. I haven't tried with the binary archive.

@nuclearsandwich
Copy link
Member

I didn't try with release-latest. I just checked with master (from source), and it was working.

Trying with release-latest from source to confirm @Mygao's findings seems prudent as well to avoid chasing shadows.

@ivanpauno
Copy link
Member

I tried from source using release-latest, and I couldn't reproduce the bug.

@nuclearsandwich nuclearsandwich added this to Proposed in Dashing Patch Release 3 via automation Aug 1, 2019
@nuclearsandwich nuclearsandwich removed this from Proposed in Dashing Patch Release 2 Aug 1, 2019
@nuclearsandwich nuclearsandwich removed this from Proposed in Dashing Patch Release 3 Sep 6, 2019
@ivanpauno
Copy link
Member

ivanpauno commented Sep 13, 2019

@Mygao could you check if this is still happening after the last patch release?

@oceanusxiv
Copy link

@ivanpauno I actually just ran into this issue and I'm using the latest debian of dashing as far as I can tell

@oceanusxiv
Copy link

oceanusxiv commented Sep 24, 2019

OK so I just found this #836 which is the PR that fixed this issue. So it looks like we should probably put this in the patch release

EDIT: oops I see that you're already aware of this. Sorry for bothering you XD

@ivanpauno
Copy link
Member

Thanks for the comment @eric1221bday, I didn't realize that #836 was a fix for this.
I created the backport PR in #869.

I'm closing this!

@nuclearsandwich
Copy link
Member

This fix was released in Dashing Patch 4 but the issue wasn't removed from the board so it got bumped forward. I'm taking it off now but the fix was released.

@nuclearsandwich nuclearsandwich removed this from Proposed in Dashing Patch Release 5 Dec 5, 2019
nnmm pushed a commit to ApexAI/rclcpp that referenced this issue Jul 9, 2022
Signed-off-by: Stephen Brawner <brawner@gmail.com>
nnmm pushed a commit to ApexAI/rclcpp that referenced this issue Jul 9, 2022
* Refactor rcl_yaml_param_parser for better testability

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* Reorder parser.c to match parser.h

Signed-off-by: Stephen Brawner <brawner@gmail.com>

squash! Reorder parser.c to match parser.h

* Refactor yaml_variant.c for deduplication

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* ADD_VALUE_TO_SIMPLE_ARRAY for deduplication

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* Remove fprintf

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* PR Fixup

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* Move headers to src directory

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* PR Fixup

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* Rebase ros2#780

Signed-off-by: Stephen Brawner <brawner@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants