fix: join the Polygon render worker in the destructor

[rolker]

Removing a Polygon layer from the layer tree freed the object while its
QtConcurrent render worker was still running — the same defect fixed for
OccupancyGrid in #209/#210. The operator triggers it by unchecking a layer.

~Polygon() now raises a shutdown_ gate, resets the subscription to stop new
callbacks at the source, then joins the running worker; polygonCallback checks
the gate before launching more work.

Two hazards specific to joining in a destructor

Pre-push review caught both — neither is in the OccupancyGrid precedent as a
visible pattern, and the first is a regression the join itself introduces:

• waitForFinished() rethrows the worker's stored exception, and a
  destructor is implicitly noexcept. frameOriginInWebMercator() throws
  tf2::TransformException on a TF cold start — routine for a layer
  auto-created for a topic whose frame isn't in TF yet — so an unguarded join
  turns that into std::terminate(), strictly worse than the race being fixed.
  processPolygon() now catches and warns, matching
  OccupancyGrid::processGrid() and GridMap::render().
• waitForFinished() may steal a still-queued runnable and run it on the
  GUI thread inside the destructor. The emit then becomes a direct call into
  updatePolygon(), parenting a new QGraphicsPolygonItem to a dying layer and
  defeating the gate. The gate is re-checked immediately before the emit.

Deliberately not replicated

The visibility gate and itemChange mirror OccupancyGrid carries are omitted:
processPolygon is a short point loop with no rasterisation cost and touches no
GUI-thread-owned state, so there is no camp#208 race to mirror.

Known residual

A callback preempted after passing the shutdown_ check can still resume
after the join returns. This is the camp#212 residual and is present in the
merged OccupancyGrid fix too — GridMap's mutex_ + shutdown_ handshake
closes it. Not fixed here to keep the pre-Shoals diff minimal; tracked in #212.

Test plan

• ./ui_ws/build.sh camp — builds clean (one pre-existing unused-variable
  warning in geometry_manager.cpp)
• ./ui_ws/test.sh camp — 293 tests, 0 errors, 0 failures, 1 skipped
• Runtime verification of layer removal is owed on the sim before the Shoals
  deployment.

Closes #213

---

Authored-By: Claude Code Agent
Model: Claude Opus 5

[Copilot]

🟡 Changes recommended

process_future_ is accessed from multiple threads (executor callback vs GUI destructor) without a mutex/handshake, which is a data race/UB and can undermine the intended teardown safety.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens the camp::ros::geometry::Polygon ROS layer teardown path so removing a Polygon layer doesn’t free the object while its QtConcurrent render worker is still running (the same class of crash previously fixed for grid layers).

Changes:

• Add ~Polygon() that gates shutdown, resets the ROS subscription, and joins the in-flight render worker.
• Add a shutdown_ gate checked in polygonCallback() and re-checked in processPolygon() before emitting to avoid teardown-time GUI mutation.
• Catch TF/transform exceptions in the worker to prevent exceptions from escaping during destructor-initiated joins.

File summaries

File	Description
src/camp_map/ros/geometry/polygon.h	Adds ~Polygon() declaration and a shutdown gate used to prevent new work during teardown.
src/camp_map/ros/geometry/polygon.cpp	Implements destructor joining, shutdown gating, and exception handling in the worker.
.agent/work-plans/issue-213/progress.md	Documents the issue context, risks, and local pre-push review notes for #213.

Review details

Suppressed comments (1)

src/camp_map/ros/geometry/polygon.cpp:53

• process_future_ is accessed concurrently from the ROS executor thread (polygonCallback: isRunning()/assignment) and the GUI thread (~Polygon: waitForFinished()). Without a mutex/handshake, this is a C++ data race/UB and can still allow a new QtConcurrent worker to be launched during teardown (same class of issue GridMap avoids by copying the pending future under a lock before waiting). Consider guarding process_future_ with a mutex and (a) re-check shutdown_ under that lock before launching work, and (b) in the destructor copy the current future under the same lock and wait on the copy outside the lock (see GridMap::~GridMap()).

    return;
  if(!process_future_.isRunning())
  {
    process_future_ = QtConcurrent::run(this, &Polygon::processPolygon, data);
  }

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

The teardown/join pattern matches the established GridMap approach and the added exception + pre-emit shutdown checks address the destructor-join hazards without introducing new races in the reviewed code paths.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite