Fix race condition (#293)

* Change mutex locker scope in SubmapsDisplay::update()

This avoids the potential unsafe situation where someone (for example,
SubmapsDisplay::Reset()) clears the trajectories vector, but update()
uses it after the mutex has been released, since the scope of the lock
was only in the try block.

Also resolve a miscellaneous signed-unsigned warning.

* Make DrawableSubmap destructor safer

Make the DrawableSubmap destructor wait for the asynchronous fetch
thread to finish - otherwise, it's possible to crash rviz by rapidly
pressing the enable checkbox of the submaps plugin. Eventually the race
condition will be triggered, and the fetch thread will write into a
deleted DrawableSubmap object, which causes a crash.

cartographer_rviz/cartographer_rviz/drawable_submap.cc

@@ -78,6 +78,9 @@ DrawableSubmap::DrawableSubmap(const int trajectory_id, const int submap_index,
 }
 
 DrawableSubmap::~DrawableSubmap() {
+  if (QueryInProgress()) {
+    rpc_request_future_.wait();
+  }
   Ogre::MaterialManager::getSingleton().remove(material_->getHandle());
   if (!texture_.isNull()) {
     Ogre::TextureManager::getSingleton().remove(texture_->getHandle());

cartographer_rviz/cartographer_rviz/submaps_display.cc

@@ -114,13 +114,13 @@ void SubmapsDisplay::processMessage(
 }
 
 void SubmapsDisplay::update(const float wall_dt, const float ros_dt) {
+  ::cartographer::common::MutexLocker locker(&mutex_);
   // Update the fading by z distance.
   try {
     const ::geometry_msgs::TransformStamped transform_stamped =
         tf_buffer_.lookupTransform(map_frame_property_->getStdString(),
                                    tracking_frame_property_->getStdString(),
                                    ros::Time(0) /* latest */);
-    ::cartographer::common::MutexLocker locker(&mutex_);
     for (auto& trajectory : trajectories_) {
       for (auto& submap : trajectory) {
         submap->SetAlpha(transform_stamped.transform.translation.z);
@@ -138,7 +138,7 @@ void SubmapsDisplay::update(const float wall_dt, const float ros_dt) {
         ++num_ongoing_requests;
       }
     }
-    for (int submap_index = trajectory.size() - 1;
+    for (int submap_index = static_cast<int>(trajectory.size()) - 1;
          submap_index >= 0 &&
          num_ongoing_requests < kMaxOnGoingRequestsPerTrajectory;
          --submap_index) {