Fix data race in NormalEstimationOMP on Windows

[7680x4320]

Problem

The function computeFeature of the class pcl::NormalEstimationOMP calls the member function computePointNormal. And this computePointNormal accesses the member variable xyz_centroid_ and covariance_matrix_.
These variables are written at the same time in using OpenMP, so they will be destroyed.

Solution

Replace with the non-member version.

[taketwo]

You are right that we should use free function, not member. However, to the best of my knowledge, this is the case already. In order to call the member function you would need to prefix with this->.

[7680x4320]

Because this-> is optional in C++, computePointNormal is the same as this->computePointNormal in this case.
We must prefix with pcl:: to call free function here.

[taketwo]

No, in our case we have a templated base class and your statement is not correct. I urge you to a) add a print statement and check which function gets called; b) read up on the topic, e.g. this stack overflow thread.

Edit: that said, I think we can merge your change since it makes the intention of the code more clear. I just wouldn't call it a "fix", rather a "style improvement".

[7680x4320]

Sorry, I wasn't aware of the case of template base class member. And sorry for my lack of explanation about my environment. As the description that you referred, a behavior of MSVC is non-comforming. It compiles computePointNormal() as this->computePointNormal(). To avoid this, I thought we should prefix with pcl::.

[taketwo]

I noticed that disclaimer about MSVC, but since the answer was given 8 years ago I assumed Microsoft had enough time to fix this. Which version do you have? And you confirmed with a print statement that the wrong function is used?

[7680x4320]

Microsoft corrected this behavior at version 15.5 (released in Dec 2017). Surely, the following code compiled by MSVC 15.9 with default compiler option prints "free". But MSVC 2015 (Update 3) will print "member" (sorry, I can't confirm actually just now) and MSVC 15.9 without /permissive- option prints "member". I don't know why but, unfortunately, this option (or related ones) seems to have been disabled automatically by MSVC 15.9 in compiling PCL.

void foo() { std::cerr << "free" << std::endl; }

template <typename T>
class A
{
public:
	void foo() { std::cerr << "member" << std::endl; }
};

template <typename T>
class B : public A<T>
{
public:
	void bar() { foo(); }
};


int main()
{
	B<pcl::Normal> b;
	b.bar();
}

[taketwo]

OK, thanks for the research, we both learned something today 😄

[SergioRAgostinho]

Thanks for clarifying things.