Simplify data_ and data_interface_ in KDTreeFlann.

KDTreeFlann::SetRawData had some confusion around data, data_, and
data_interface_. As a result, the data from the parameter was copied to
the member std::vector, but the interface (which takes an Eigen::Map)
used the data passed as an argument to the method.
As a result, the searches risked to be run on a dangling pointer,
instead of the intended internal storage.

However, rather than adjusting the pointer for Eigen::Map, I preferred
simplifying the code, and copy the argument to an Eigen::MatrixXd.

CHANGELOG.md

@@ -32,6 +32,7 @@
 -   Fix regression in printing cuda tensor from PR #6444 🐛
 -   Add Python pathlib support for file IO (PR #6619)
 -   Fix log error message for `probability` argument validation in `PointCloud::SegmentPlane` (PR #6622)
+-   Fix KDTreeFlann possibly using a dangling pointer instead of internal storage and simplified its members (PR #6734)
 
 ## 0.13
 

cpp/open3d/geometry/KDTreeFlann.cpp

@@ -97,8 +97,7 @@ int KDTreeFlann::SearchKNN(const T &query,
     // This is optimized code for heavily repeated search.
     // Other flann::Index::knnSearch() implementations lose performance due to
     // memory allocation/deallocation.
-    if (data_.empty() || dataset_size_ <= 0 ||
-        size_t(query.rows()) != dimension_ || knn < 0) {
+    if (data_.size() == 0 || query.rows() != data_.rows() || knn < 0) {
         return -1;
     }
     indices.resize(knn);
@@ -121,8 +120,7 @@ int KDTreeFlann::SearchRadius(const T &query,
     // Since max_nn is not given, we let flann to do its own memory management.
     // Other flann::Index::radiusSearch() implementations lose performance due
     // to memory management and CPU caching.
-    if (data_.empty() || dataset_size_ <= 0 ||
-        size_t(query.rows()) != dimension_) {
+    if (data_.size() == 0 || query.rows() != data_.rows()) {
         return -1;
     }
     std::vector<nanoflann::ResultItem<Eigen::Index, double>> indices_dists;
@@ -148,8 +146,7 @@ int KDTreeFlann::SearchHybrid(const T &query,
     // It is also the recommended setting for search.
     // Other flann::Index::radiusSearch() implementations lose performance due
     // to memory allocation/deallocation.
-    if (data_.empty() || dataset_size_ <= 0 ||
-        size_t(query.rows()) != dimension_ || max_nn < 0) {
+    if (data_.size() == 0 || query.rows() != data_.rows() || max_nn < 0) {
         return -1;
     }
     distance2.resize(max_nn);
@@ -166,18 +163,12 @@ int KDTreeFlann::SearchHybrid(const T &query,
 }
 
 bool KDTreeFlann::SetRawData(const Eigen::Map<const Eigen::MatrixXd> &data) {
-    dimension_ = data.rows();
-    dataset_size_ = data.cols();
-    if (dimension_ == 0 || dataset_size_ == 0) {
+    if (data.size() == 0) {
         utility::LogWarning("[KDTreeFlann::SetRawData] Failed due to no data.");
         return false;
     }
-    data_.resize(dataset_size_ * dimension_);
-    memcpy(data_.data(), data.data(),
-           dataset_size_ * dimension_ * sizeof(double));
-    data_interface_.reset(new Eigen::Map<const Eigen::MatrixXd>(data));
-    nanoflann_index_.reset(
-            new KDTree_t(dimension_, std::cref(*data_interface_), 15));
+    data_ = data;
+    nanoflann_index_ = std::make_unique<KDTree_t>(data_.rows(), data_, 15);
     nanoflann_index_->index_->buildIndex();
     return true;
 }

cpp/open3d/geometry/KDTreeFlann.h

@@ -98,16 +98,13 @@ class KDTreeFlann {
 
 protected:
     using KDTree_t = nanoflann::KDTreeEigenMatrixAdaptor<
-            Eigen::Map<const Eigen::MatrixXd>,
+            const Eigen::MatrixXd,
             -1,
             nanoflann::metric_L2,
             false>;
 
-    std::vector<double> data_;
-    std::unique_ptr<Eigen::Map<const Eigen::MatrixXd>> data_interface_;
+    Eigen::MatrixXd data_;
     std::unique_ptr<KDTree_t> nanoflann_index_;
-    size_t dimension_ = 0;
-    size_t dataset_size_ = 0;
 };
 
 }  // namespace geometry