haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix triangulationValid check race for zone fill 

The m_triangulationValid flag is used in several places without holding
the mutex, thus it should only ever be set when the triangulation is
guaranteed to be valid.

This can either be done by protecting both data and flag by the same
mutex, or updating the flag only after the triangulation has finished.

Also fix the case when the triangulation actually fails, the flag should
not be set in this case.

While at it, simplify the recalculation check. Only if both the
triangulation is valid, and the data hash is unchanged the recalculation
can be skipped - this is typically the case when two threads try to
update the cache concurrently, the second one will block at the mutex,
and will see the valid data after the first thread has finished.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/17180
This commit is contained in:
StefanBruens 2024-03-14 22:26:23 +00:00 committed by Seth Hillbrand
parent 91b850d8df
commit e07b4ce8e4
2 changed files with 21 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
libs/kimath/include/geometry/shape_poly_set.h
View File

@ -28,6 +28,7 @@
#ifndef __SHAPE_POLY_SET_H
#define __SHAPE_POLY_SET_H
#include <atomic>
#include <cstdio>
#include <deque> // for deque
#include <iosfwd> // for string, stringstream
@ -1538,7 +1539,7 @@ protected:
std::vector<POLYGON> m_polys;
std::vector<std::unique_ptr<TRIANGULATED_POLYGON>> m_triangulatedPolys;
bool m_triangulationValid = false;
std::atomic<bool> m_triangulationValid = false;
std::mutex m_triangulationMutex;
private:

38
libs/kimath/src/geometry/shape_poly_set.cpp
View File

@ -2883,7 +2883,7 @@ SHAPE_POLY_SET &SHAPE_POLY_SET::operator=( const SHAPE_POLY_SET& aOther )
}
m_hash = aOther.m_hash;
m_triangulationValid = aOther.m_triangulationValid;
m_triangulationValid = aOther.m_triangulationValid.load();
return *this;
}
@ -2985,25 +2985,16 @@ void SHAPE_POLY_SET::cacheTriangulation( bool aPartition, bool aSimplify,
std::vector<std::unique_ptr<TRIANGULATED_POLYGON>>* aHintData )
{
std::unique_lock<std::mutex> lock( m_triangulationMutex );
bool recalculate = !m_hash.IsValid();
MD5_HASH hash;
if( !m_triangulationValid )
recalculate = true;
if( !recalculate )
if( m_triangulationValid && m_hash.IsValid() )
{
hash = checksum();
if( m_hash != hash )
{
m_hash = hash;
recalculate = true;
}
if( m_hash == checksum() )
return;
}
if( !recalculate )
return;
// Invalidate, in case anything goes wrong below
m_triangulationValid = false;
m_hash.SetValid( false );
auto triangulate =
[]( SHAPE_POLY_SET& polySet, int forOutline,
@ -3054,7 +3045,6 @@ void SHAPE_POLY_SET::cacheTriangulation( bool aPartition, bool aSimplify,
};
m_triangulatedPolys.clear();
m_triangulationValid = true;
if( aPartition )
{
@ -3081,6 +3071,12 @@ void SHAPE_POLY_SET::cacheTriangulation( bool aPartition, bool aSimplify,
{
wxLogTrace( TRIANGULATE_TRACE, "Failed to triangulate partitioned polygon %d", ii );
}
else
{
m_hash = checksum();
// Set valid flag only after everything has been updated
m_triangulationValid = true;
}
}
}
else
@ -3095,9 +3091,13 @@ void SHAPE_POLY_SET::cacheTriangulation( bool aPartition, bool aSimplify,
{
wxLogTrace( TRIANGULATE_TRACE, "Failed to triangulate polygon" );
}
else
{
m_hash = checksum();
// Set valid flag only after everything has been updated
m_triangulationValid = true;
}
}
m_hash = checksum();
}