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

Removed nullptr access from Timer().hasStarted() #1541

Merged
merged 1 commit into from
Jan 2, 2019
Merged

Removed nullptr access from Timer().hasStarted() #1541

merged 1 commit into from
Jan 2, 2019

Conversation

kunaltyagi
Copy link
Contributor

Description

This is a combo bug report and resolution.

Currently calling hasStarted() for a timer which hasn't been initialized properly just results in nullptr access. I added a short-circuit to prevent that from occurring.

Scope

This change needs to be backported to Kinetic and Lunar (being maintained as of now). Doesn't affect Indigo

Details

The reported error post crash is:

/usr/include/boost/smart_ptr/shared_ptr.hpp:648: typename boost::detail::sp_member_access<T>::type boost::shared_ptr<T>::operator->() const [with T = ros::Timer::Impl; typename boost::detail::sp_member_access<T>::type = ros::Timer::Impl*]: Assertion `px != 0' failed.

@kunaltyagi
Copy link
Contributor Author

Weird. The tests should not be failing

@mikepurvis
Copy link
Member

This looks like a reasonable and safe fix, thanks!

@ros-pull-request-builder retest this please

@dirk-thomas
Copy link
Member

Thanks for the fix.

@dirk-thomas dirk-thomas merged commit fe9479c into ros:melodic-devel Jan 2, 2019
meyerj added a commit to meyerj/ros_comm that referenced this pull request Jan 4, 2019
meyerj added a commit to meyerj/ros_comm that referenced this pull request Jan 31, 2019
dirk-thomas pushed a commit that referenced this pull request Jan 31, 2019
* roscpp: copy hasStarted() member function from ros::Timer to ros::WallTimer and ros::SteadyTimer

ros::Timer::hasStarted() has been added in #1464. The same member function should exist in the other
two timer implementations, too, for completeness.

* Check for nullptr in WallTimer::hasStarted() and SteadyTimer::hasStarted()

Analogous to fe9479c (#1541).
tahsinkose pushed a commit to tahsinkose/ros_comm that referenced this pull request Apr 15, 2019
tahsinkose pushed a commit to tahsinkose/ros_comm that referenced this pull request Apr 15, 2019
* roscpp: copy hasStarted() member function from ros::Timer to ros::WallTimer and ros::SteadyTimer

ros::Timer::hasStarted() has been added in ros#1464. The same member function should exist in the other
two timer implementations, too, for completeness.

* Check for nullptr in WallTimer::hasStarted() and SteadyTimer::hasStarted()

Analogous to fe9479c (ros#1541).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants