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

free valid_transitions for all states #509

Closed
wants to merge 31 commits into from
Closed

free valid_transitions for all states #509

wants to merge 31 commits into from

Conversation

vmayoral
Copy link
Member

transition_map->states[i].valid_transitions wasn't being release which was
leaking about 800 bytes per instantiation.

See aliasrobotics/RVD#333. There's a pending quality-related leak in case someone wants to look at it.

@vmayoral
free valid_transitions for all states
091a40c 
transition_map->states[i].valid_transitions wasn't being release which was
leaking about 800 bytes per instantiation.

See aliasrobotics/RVD#333

Signed-off-by: Víctor Mayoral Vilches <v.mayoralv@gmail.com>
@vmayoral vmayoral mentioned this pull request Sep 25, 2019
Closed
hidmic and others added 2 commits September 25, 2019 14:57
@hidmic
Parse CLI parameters and YAML files (ros2#508)
9196cb8 
* Enable incremental parameter yaml file parsing.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

* Parse CLI parameter files and overrides in rcl.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

* Address peer review comments.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@ivanpauno
Delete rcl_impl_getenv, replaced by rcutils_get_env (ros2#502)
3e0efce 
Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>
clalancette
clalancette approved these changes Sep 26, 2019
View reviewed changes
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Good catch. I'll approve and run CI on it; assuming it passes, I'll merge. Thanks.

@clalancette
Copy link
Contributor

CI:

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

@clalancette
Copy link
Contributor

@vmayoral The test failures in the Linux CI all seem new with this CI; they didn't happen in the nightlies: https://ci.ros2.org/view/nightly/job/nightly_linux_debug/1325/ . Thoughts?

@vmayoral
Copy link
Member Author

Our dynamic testing is running on top of the latest ros2 repos, I believe that corresponds with your nightly builds though I'm not sure.

This flaw was detected using Google Sanitizers thereby I don't see any reason not to appear in your builds as well. It should IMHO.

As for the bug, I started researching this because it was leaking 1MB+ on initialization. It looked like a potential vulnerability but I don't believe it now since it'd be hard to exploit beyond forcing additional initializations. Memory should be released though. The patch is just a proposal but feel free to change it if you guys would prefer to address this differently.

@clalancette
Changelog.
6b865af 
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
0.8.0
d655147
@clalancette
Copy link
Contributor

This flaw was detected using Google Sanitizers thereby I don't see any reason not to appear in your builds as well. It should IMHO.

I totally agree; this seems like a bug. And your fix seems reasonable. But it is causing test failures when I ran it through CI, so I think this needs a closer look.

@clalancette
Fixups.
05a6e8a 
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Copy link
Contributor

clalancette commented Oct 1, 2019 •
edited
Loading

New CI: * Linux Build Status

@mabelzhang mabelzhang added enhancement New feature or request in progress Actively being worked on (Kanban column) and removed enhancement New feature or request labels Oct 3, 2019
clalancette and others added 11 commits October 8, 2019 08:05
@clalancette
Switch the default logging implementation to spdlog.
d330c92 
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Print out a much more useful error message on failure.
2051704 
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Fix cpplint/uncrustify failure.
a251a84 
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Changelog.
e261e5c 
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
0.8.1
0198ffe
@wjwwood
add mechanism to pass rmw impl specific payloads during pub/sub creat…
e673988 
…ion (ros2#513)

* add optional rmw payload to rcl options for pub and sub

Signed-off-by: William Woodall <william@osrfoundation.org>

* move ignore_local_publications into rmw options structure for subs

Signed-off-by: William Woodall <william@osrfoundation.org>
@jacobperron
Remove vestigial references to rcl_ok() (ros2#516)
d9e4c2e 
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
[rcl_action] Correct action server documentation (ros2#519)
b7610d9 
rcl_action_process_cancel_request() will never return RCL_RET_ACTION_SERVER_TAKE_FAILED.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Update rcl_node_init docstring (ros2#517)
5fb711c 
Include rcl_context_t in the example usage.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@hidmic
Handle zero non-ROS specific args properly in rcl_remove_ros_arguments (
1a840d1 
ros2#518)

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@fujitatomoya @ivanpauno
Not creating rosout publisher instance unless required. (ros2#514)
f92e52a 
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
@mabelzhang mabelzhang added the enhancement New feature or request label Oct 17, 2019
Karsten1987 and others added 11 commits October 18, 2019 14:51
@Karsten1987
Zero copy api (ros2#506)
f4554f9 
* allocate loaned message

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* can_loan_messages for subscription

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* first draft of loaned message sequence

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* take loaned message

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* borrow/return & take/release

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* introduce rmw_publish_loaned_message

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* const correct publish

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* fix typo

Signed-off-by: Knese Karsten <karsten@openrobotics.org>

* address review

Signed-off-by: Karsten Knese <karsten@openrobotics.org>
@iluetkeb @hidmic
Add initial instrumentation (ros2#473)
60bdbd2 
Signed-off-by: Christophe Bedard <fixed-term.christophe.bourquebedard@de.bosch.com>
@BMarchi @hidmic
Add localhost option to node creation (ros2#520)
a073507 
* Add localhost option to node creation

Signed-off-by: Brian Ezequiel Marchi <brian.marchi65@gmail.com>

* Bring localhost function from rmw

Signed-off-by: Brian Ezequiel Marchi <brian.marchi65@gmail.com>

* Satisfy uncrustify and address pr comments

Signed-off-by: Brian Ezequiel Marchi <brian.marchi65@gmail.com>
@pbaughman @sloretz
ready_fn and self.proc_info will be removed in future (ros2#522)
7859398 
Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
@clalancette
Remove the maximum string size. (ros2#524)
cdf1b4d 
It wasn't preventing any allocations from happening, so it
doesn't seem to serve much purpose.  Also remove the tests
for the maximum string size.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@Karsten1987 @mjcarroll
use return_loaned_message_from (ros2#523)
68c93c8 
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
@clalancette
Remove the prototype from rcl_impl_getenv. (ros2#525)
53e88d8 
The implementation was removed in 3e0efce

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@rotu @dirk-thomas
Specify test working directory (ros2#529)
c173935 
Avert breakage from ament/ament_cmake#206
Signed-off-by: Dan Rose <dan@digilabs.io>
@sloretz
0.8.2
3d3b77c 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@hidmic
Support CLI parameter overrides using dots instead of slashes. (ros2#530
513a185 
)

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@sloretz
0.8.3
9034c40 
Signed-off-by: Shane Loretz <sloretz@openrobotics.org>
@vmayoral vmayoral mentioned this pull request Nov 21, 2019
Closed
34 tasks
@dirk-thomas
Copy link
Member

@vmayoral @clalancette What is the status of this patch? On which side is the ball to follow up on this?

@vmayoral
Copy link
Member Author

@dirk-thomas It's up to you guys.

@dirk-thomas
Copy link
Member

Latest CI: Build Status

@vmayoral Please rebase the branch against the latest state of the target branch in order to run CI.

vmayoral and others added 3 commits November 22, 2019 09:24
@vmayoral
free valid_transitions for all states
d9a282c 
transition_map->states[i].valid_transitions wasn't being release which was
leaking about 800 bytes per instantiation.

See aliasrobotics/RVD#333

Signed-off-by: Víctor Mayoral Vilches <v.mayoralv@gmail.com>
@clalancette @vmayoral
Fixups.
40b7903 
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@vmayoral
Merge branch 'fix-bug-rcl' of https://github.com/vmayoral/rcl into fi…
39b8e07 
…x-bug-rcl
@vmayoral vmayoral mentioned this pull request Nov 22, 2019
Merged
@vmayoral vmayoral closed this Nov 22, 2019
@vmayoral
Copy link
Member Author

Follows at #537

@vmayoral vmayoral mentioned this pull request Jan 16, 2020
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clalancette clalancette clalancette approved these changes

@jacobperron jacobperron jacobperron left review comments

Assignees
No one assigned
Labels
enhancement New feature or request in progress Actively being worked on (Kanban column)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet