Missing copy-on-write protection in World::moveShapesInObject and setSubframesOfObject

Open Beginner friendly
  • Is something wrong?
#3,940 2 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

  • #3957 by @paneendrakumar0 — closed without merging

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
76/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
cpp
Domain
backend

Research direction

Read moveit_core/collision_detection/src/world.cpp, comparing moveShapesInObject() and setSubframesOfObject() with moveShapeInObject(), which already calls ensureUnique(). Add regression tests that copy a World, update shape poses or subframes in one copy, and check that the other copy retains its original state. Done means both methods preserve copy-on-write isolation and the regression tests pass.

Written by the indexing model from the issue text.

Description

bug
Description

Description

In moveit_core/collision_detection/src/world.cpp, two methods modify the internal state of a world object without first calling ensureUnique():

  • World::moveShapesInObject() (around line 266)
  • World::setSubframesOfObject() (around line 369)

The ensureUnique() helper implements copy-on-write protection by creating an independent object instance when the existing object is shared.

A similar method, World::moveShapeInObject(), correctly calls ensureUnique(it->second) before modifying the object's shape poses:

ensureUnique(it->second);
ASSERT_ISOMETRY(shape_pose)
it->second->shape_poses_[i] = shape_pose;
it->second->global_shape_poses_[i] = it->second->pose_ * shape_pose;

However, moveShapesInObject() directly modifies the stored object:

for (std::size_t i = 0; i < shape_poses.size(); ++i)
{
ASSERT_ISOMETRY(shape_poses[i])
it->second->shape_poses_[i] = shape_poses[i];
it->second->global_shape_poses_[i] = it->second->pose_ * shape_poses[i];
}

Similarly, setSubframesOfObject() updates the object directly:

obj_pair->second->subframe_poses_ = subframe_poses;
obj_pair->second->global_subframe_poses_ = subframe_poses;
updateGlobalPosesInternal(obj_pair->second, false, true);

Neither method calls ensureUnique() before modifying the object.

If multiple World instances share the same underlying object, these operations may unintentionally modify the state visible to other world instances, violating copy-on-write isolation.

Suggested Fix

Call ensureUnique() before mutating the object in both methods, consistent with moveShapeInObject().

Add regression tests verifying that changes made to one World instance do not affect another World instance sharing the original object.

ROS Distro

Humble

OS and version

Ubuntu 22.04

Source or binary build?

Source

If binary, which release version?

No response

If source, which branch?

No response

Which RMW are you using?

None

Steps to Reproduce
  1. Create a collision_detection::World instance.
  2. Add a collision object containing multiple shapes and a subframe.
  3. Copy the World instance so that both Worlds initially share the same underlying object.
  4. Record the shape poses and subframe transforms in the copied World.
  5. Call moveShapesInObject() on the original World with updated shape poses.
  6. Check whether the corresponding shape poses in the copied World have also changed.
  7. Repeat the experiment using setSubframesOfObject() to update the subframe transforms.
  8. Compare the states of both World instances after each operation.

These are proposed regression-test steps. The suspected cross-World state modification has not yet been verified at runtime.

Expected behavior

When a World instance is copied, the two instances may initially share object storage for efficiency.

However, modifying the shape poses or subframe transforms in one World should not affect the corresponding objects in the other World.

Both moveShapesInObject() and setSubframesOfObject() should preserve copy-on-write isolation, consistently with other World mutation methods.

Actual behavior

Source inspection shows that moveShapesInObject() and setSubframesOfObject() directly modify the stored World object without calling ensureUnique().

Other World mutation methods, such as moveShapeInObject(), explicitly call ensureUnique() before modifying object state.

If the object is shared by multiple World instances, the missing copy-on-write protection may cause modifications in one World to become visible in another.

This behavior has not yet been confirmed with a runtime regression test.

Backtrace or Console output

No response

Dominant language
C++
Stars
2.1k
Forks
816
Avg merge
1d 2h
Merged PRs (30d)
4

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from moveit/moveit2

All issues in moveit/moveit2

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.