search

AcademySoftwareFoundation / openvdb

OpenVDB - Sparse volume data structure and tools
http://www.openvdb.org/
Mozilla Public License 2.0
2.62k stars 647 forks source link

ThreadSanitizer issue in fillWithSpheres() function #1739

Closed jdumas closed 8 months ago

jdumas commented 9 months ago

Environment

Operating System: macOS/Linux Version / Commit SHA: OpenVDB 11.0.0, OneTBB v2021.11.0 Other: C++17

Describe the bug

While running ThreadSanitizer on a unit test with the fillWithSpheres() function, I think I uncovered a potential thread-safety issue at the following location: https://github.com/AcademySoftwareFoundation/openvdb/blob/605970fc1e374f4a84eaacc836f64a93e4f298a9/openvdb/openvdb/tools/VolumeToSpheres.h#L553-L554

According to the parallel_reduce documentation, the operator() can run concurrently with the split constructor. This this update to mRadius and mIndex is potentially unsafe, and should use an atomic. I don't think it will change the result, but it will make TSan happy at least.

For reference here is the full stack-trace on the first TSan issue I ran into: tsan_log.txt

Idclip commented 8 months ago

Thanks @jdumas, looks like a legit issue to me too, PR here https://github.com/AcademySoftwareFoundation/openvdb/pull/1746