Skip to content

Commit

Permalink
fix data races (#1019)
Browse files Browse the repository at this point in the history
  • Loading branch information
pca006132 authored Nov 4, 2024
1 parent f11ed8d commit 4076b14
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 8 deletions.
9 changes: 8 additions & 1 deletion include/manifold/parallel.h
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,14 @@ struct SortedRange {
: input(input), tmp(tmp), offset(offset), length(length) {}
SortedRange(SortedRange<T, SizeType> &r, tbb::split)
: input(r.input), tmp(r.tmp) {}
void operator()(const tbb::blocked_range<SizeType> &range) {
// FIXME: no idea why thread sanitizer reports data race here
#if defined(__has_feature)
#if __has_feature(thread_sanitizer)
__attribute__((no_sanitize("thread")))
#endif
#endif
void
operator()(const tbb::blocked_range<SizeType> &range) {
SortedRange<T, SizeType> rhs(input, tmp, range.begin(),
range.end() - range.begin());
rhs.inTmp =
Expand Down
7 changes: 7 additions & 0 deletions src/boolean_result.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,13 @@ struct EdgePos {
bool isStart;
};

// thread sanitizer doesn't really know how to check when there are too many
// mutex
#if defined(__has_feature)
#if __has_feature(thread_sanitizer)
__attribute__((no_sanitize("thread")))
#endif
#endif
void AddNewEdgeVerts(
// we need concurrent_map because we will be adding things concurrently
concurrent_map<int, std::vector<EdgePos>> &edgesP,
Expand Down
10 changes: 6 additions & 4 deletions src/impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ struct CoplanarEdge {
const double epsilon;
const double tolerance;

// FIXME: race condition
void operator()(const int edgeIdx) {
const Halfedge edge = halfedge[edgeIdx];
const Halfedge pair = halfedge[edge.pairedHalfedge];
Expand Down Expand Up @@ -136,8 +135,9 @@ struct CoplanarEdge {
vec3 normal = la::cross(jointVec, edgeVec);
const double area = la::length(normal);
const double areaPair = la::length(la::cross(pairVec, jointVec));
triArea[edgeFace] = area;
triArea[pairFace] = areaPair;

// make sure we only write this once
if (edgeIdx % 3 == 0) triArea[edgeFace] = area;
// Don't link degenerate triangles
if (area < length * epsilon || areaPair < lengthPair * epsilon) return;

Expand All @@ -158,7 +158,9 @@ struct CheckCoplanarity {

void operator()(int tri) {
const int component = (*components)[tri];
const int referenceTri = comp2tri[component];
const int referenceTri =
reinterpret_cast<std::atomic<int>*>(&comp2tri[component])
->load(std::memory_order_relaxed);
if (referenceTri < 0 || referenceTri == tri) return;

const vec3 origin = vertPos[halfedge[3 * referenceTri].startVert];
Expand Down
9 changes: 7 additions & 2 deletions src/properties.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ struct CurvatureAngles {
struct UpdateProperties {
VecView<ivec3> triProp;
VecView<double> properties;
VecView<uint8_t> counters;

VecView<const double> oldProperties;
VecView<const Halfedge> halfedge;
Expand All @@ -100,7 +101,6 @@ struct UpdateProperties {
const int gaussianIdx;
const int meanIdx;

// FIXME: race condition
void operator()(const size_t tri) {
for (const int i : {0, 1, 2}) {
const int vert = halfedge[3 * tri + i].startVert;
Expand All @@ -109,6 +109,10 @@ struct UpdateProperties {
}
const int propVert = triProp[tri][i];

auto old = std::atomic_exchange(
reinterpret_cast<std::atomic<uint8_t>*>(&counters[propVert]), 1);
if (old == 1) return;

for (int p = 0; p < oldNumProp; ++p) {
properties[numProp * propVert + p] =
oldProperties[oldNumProp * propVert + p];
Expand Down Expand Up @@ -290,10 +294,11 @@ void Manifold::Impl::CalculateCurvature(int gaussianIdx, int meanIdx) {
meshRelation_.triProperties.resize(NumTri());
}

const Vec<uint8_t> counters(NumPropVert(), 0);
for_each_n(
policy, countAt(0_uz), NumTri(),
UpdateProperties({meshRelation_.triProperties, meshRelation_.properties,
oldProperties, halfedge_, vertMeanCurvature,
counters, oldProperties, halfedge_, vertMeanCurvature,
vertGaussianCurvature, oldNumProp, numProp, gaussianIdx,
meanIdx}));
}
Expand Down
8 changes: 7 additions & 1 deletion src/smoothing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,14 @@ Vec<int> Manifold::Impl::VertFlatFace(const Vec<bool>& flatFaces) const {

Vec<int> Manifold::Impl::VertHalfedge() const {
Vec<int> vertHalfedge(NumVert());
Vec<uint8_t> counters(NumVert(), 0);
for_each_n(autoPolicy(halfedge_.size(), 1e5), countAt(0), halfedge_.size(),
[&vertHalfedge, this](const int idx) {
[&vertHalfedge, &counters, this](const int idx) {
auto old = std::atomic_exchange(
reinterpret_cast<std::atomic<uint8_t>*>(
&counters[halfedge_[idx].startVert]),
1);
if (old == 1) return;
// arbitrary, last one wins.
vertHalfedge[halfedge_[idx].startVert] = idx;
});
Expand Down

0 comments on commit 4076b14

Please sign in to comment.