FLANN Index params should be copied to avoid dangling pointer (Bug #2050)
| Status: | Open | Start date: | 2012-06-14 |
|---|---|---|---|
| Priority: | Normal | Due date: | |
| Assignee: |  Vadim Pisarevsky |
% Done: | 0% |
| Category: | documentation | ||
| Target version: | - | ||
| Affected version: | Operating System: | ||
| Difficulty: | HW Platform: | ||
| Pull request: |
Description
Hi,
in my program I was creating a flann index in one method (assigned to a member variable of type cv::flann::Index), let's say method* A()*, so that I can use it for searching in another method, say B(). Example:
flannIndex = cv::flann::Index(myData32f, cv::flann::KDTreeIndexParams());
However, searching for the k-NN in B() would cause an exception, because the index parameters are deleted at the point when A() exits. I.e.
cv::flann::IndexParams::~IndexParams()
{
delete &get_params(*this);
}
is called, which causes a dangling pointer at:
void runKnnSearch_(void* index, const Mat& query, Mat& indices, Mat& dists,
int knn, const SearchParams& params)
{
[some code left out]
((IndexType*)index)->knnSearch(_query, _indices, _dists, knn,
* (const ::cvflann::SearchParams&)get_params(params)*);
}
The current way the FLANN wrapper is coded forces one to either create and use the index in the same method, or to create the index parameter at a safe place (e.g. member variable of your class) such that it is not deleted prematurely. It would be better instead, if the CTOR of cv::flann::Index actually copies the parameter!
Associated revisions
Merge pull request #2050 from asmorkalov:android_mk_fix2
History
Updated by Kirill Kornyakov over 12 years ago
BTW, are you sure that this is an OpenCV bug, and not the FLANN itself? In the latter case you should contact original authors of the library...
- Category set to flann
Updated by Marius Shekow over 12 years ago
Well, the get_params() function essentially converts the cv:flann:*IndexParams to the flann-native index-parameters object. The deletion of the cv:flann:*IndexParams object when getting out of scope causes the dangling pointer, so I'd reckon it's a OpenCV problem.
Also, I'd like to note that it really seems to be only possible to define the cv::flann::Index object in one method but use it in another, if:- You create the cv::flann::Index on the heap, not the stack
- The cv::Mat data that is indexed is available throughout the lifetime (again, why does the wrapper not internally store the cv::Mat object somewhere such that this is not necessary?)
- The IndexParam object is permanently available (as already noted)
Updated by Vadim Kantorov over 12 years ago
Marius Shekow has a seemingly complete description.
I just bumped into this bug. Sth like:
Mat features(...);
cv::flann::KDTreeIndexParams indexParams(5);
cv::flann::Index index(features, indexParams);
index.knnSearch(...);
works. While
cv::flann::Index BuildIndex(Mat features)
{
cv::flann::KDTreeIndexParams indexParams(5);
cv::flann::Index index(features, indexParams);
return index;
}
Mat features(...);
cv::flann::Index index = BuildIndex(features);
index.knnSearch(...);
does not because a good copy constructor of cv::flann is not implemented. It would be better to have it somewhere very clear in the documentation, or somehow to notify the user when they try to copy the Index object.
Updated by Andrey Kamaev over 12 years ago
- Assignee set to Vadim Pisarevsky
Updated by Andrey Kamaev over 12 years ago
- Category changed from flann to documentation
Updated by Vadim Kantorov over 12 years ago
I guess it would be much better to provide a better wrapper interface for FLANN which will hold a reference to the "features" matrix (via a Mat object, not just a pointer to float array) and to the "params" object (via a shared ptr). The current behavior is confusing and fixing documentation should only be a partial solution from my point of view.
Updated by Maksim Shabunin over 9 years ago
Issue has been transferred to GitHub: https://github.com/Itseez/opencv/issues/4376