Accepting request 1158254 from electronics

OBS-URL: https://build.opensuse.org/request/show/1158254
OBS-URL: https://build.opensuse.org/package/show/openSUSE:Factory/kicad?expand=0&rev=62
This commit is contained in:
Ana Guerrero 2024-03-15 19:31:47 +00:00 committed by Git OBS Bridge
commit 5c5ed210c2
6 changed files with 209 additions and 14 deletions

View File

@ -0,0 +1,133 @@
From 1c459e9a67151a6f028ac3a108b338e850126bfb Mon Sep 17 00:00:00 2001
From: StefanBruens <stefan.bruens@rwth-aachen.de>
Date: Wed, 13 Mar 2024 21:43:12 +0000
Subject: [PATCH] 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 #17180
---
libs/kimath/include/geometry/shape_poly_set.h | 3 +-
libs/kimath/src/geometry/shape_poly_set.cpp | 38 +++++++++----------
2 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/libs/kimath/include/geometry/shape_poly_set.h b/libs/kimath/include/geometry/shape_poly_set.h
index ac5b2325744..e3d1ac18f35 100644
--- a/libs/kimath/include/geometry/shape_poly_set.h
+++ b/libs/kimath/include/geometry/shape_poly_set.h
@@ -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:
diff --git a/libs/kimath/src/geometry/shape_poly_set.cpp b/libs/kimath/src/geometry/shape_poly_set.cpp
index dacc1e8a07c..da379ad309a 100644
--- a/libs/kimath/src/geometry/shape_poly_set.cpp
+++ b/libs/kimath/src/geometry/shape_poly_set.cpp
@@ -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();
}
--
GitLab

53
fix_zone_fill_race.patch Normal file
View File

@ -0,0 +1,53 @@
From 81cb6d0c3fb92dd15f0a0e0d2d32337be1617399 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <seth@kipro-pcb.com>
Date: Wed, 13 Mar 2024 10:33:43 -0700
Subject: [PATCH] Fix race condition in zone fill
When checking collisions, the SHAPE_POLY_SET::Collide() routine is not
const because it will regenerate the triangulation cache if it is out of
date (using a const_cast, grrr). This sidesteps the issue by assigning
a mutex to the triangulation caching
Fixes https://gitlab.com/kicad/code/kicad/-/issues/17180
---
libs/kimath/include/geometry/shape_poly_set.h | 4 +++-
libs/kimath/src/geometry/shape_poly_set.cpp | 1 +
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/libs/kimath/include/geometry/shape_poly_set.h b/libs/kimath/include/geometry/shape_poly_set.h
index 9ba7b5407b0..ac5b2325744 100644
--- a/libs/kimath/include/geometry/shape_poly_set.h
+++ b/libs/kimath/include/geometry/shape_poly_set.h
@@ -32,6 +32,7 @@
#include <deque> // for deque
#include <iosfwd> // for string, stringstream
#include <memory>
+#include <mutex>
#include <set> // for set
#include <stdexcept> // for out_of_range
#include <stdlib.h> // for abs
@@ -1537,7 +1538,8 @@ protected:
std::vector<POLYGON> m_polys;
std::vector<std::unique_ptr<TRIANGULATED_POLYGON>> m_triangulatedPolys;
- bool m_triangulationValid = false;
+ bool m_triangulationValid = false;
+ std::mutex m_triangulationMutex;
private:
MD5_HASH m_hash;
diff --git a/libs/kimath/src/geometry/shape_poly_set.cpp b/libs/kimath/src/geometry/shape_poly_set.cpp
index 407c43b9c6e..dacc1e8a07c 100644
--- a/libs/kimath/src/geometry/shape_poly_set.cpp
+++ b/libs/kimath/src/geometry/shape_poly_set.cpp
@@ -2984,6 +2984,7 @@ static SHAPE_POLY_SET partitionPolyIntoRegularCellGrid( const SHAPE_POLY_SET& aP
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;
--
GitLab

View File

@ -1,3 +0,0 @@
version https://git-lfs.github.com/spec/v1
oid sha256:aee1630aac44ab181cedd293a54c339f623361cb6d5b374c27ca5389f80a28b6
size 72430414

3
kicad-8.0.1.tar.bz2 Normal file
View File

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:7ea3f18650ce1b5359c685d10245efd95f87fa42ee52a1585bbc22d957617928
size 72453098

View File

@ -1,3 +1,12 @@
-------------------------------------------------------------------
Fri Mar 15 11:10:26 UTC 2024 - Stefan Brüns <stefan.bruens@rwth-aachen.de>
- update to 8.0.1:
See https://www.kicad.org/blog/2024/03/KiCad-8.0.1-Release/ for details
- Fix some race condition, causing e.g. fails in qa_pcbnew:
* fix_zone_fill_race.patch
* 0001-Fix-triangulationValid-check-race-for-zone-fill.patch
-------------------------------------------------------------------
Sun Feb 25 07:41:33 UTC 2024 - Stefan Brüns <stefan.bruens@rwth-aachen.de>

View File

@ -20,14 +20,18 @@
# symbol libraries from version 8.0.0
%define compatversion 8.0.0
Name: kicad
Version: 8.0.0
%define file_version 8.0.0
Version: 8.0.1
%define file_version 8.0.1
Release: 0
Summary: EDA software suite for the creation of schematics and PCB
License: AGPL-3.0-or-later AND GPL-3.0-or-later
Group: Productivity/Scientific/Electronics
URL: https://www.kicad.org
Source: https://gitlab.com/kicad/code/kicad/-/archive/%{file_version}/kicad-%{file_version}.tar.bz2
# PATCH-FIX-UPSTREAM
Patch1: https://gitlab.com/kicad/code/kicad/-/commit/81cb6d0c3fb92dd15f0a0e0d2d32337be1617399.patch#/fix_zone_fill_race.patch
# PATCH-FIX-UPSTREAM
Patch2: https://gitlab.com/kicad/code/kicad/-/commit/1c459e9a67151a6f028ac3a108b338e850126bfb.patch#/0001-Fix-triangulationValid-check-race-for-zone-fill.patch
BuildRequires: cmake >= 3.16
BuildRequires: fdupes
@ -185,17 +189,13 @@ chmod -x %{buildroot}%{_datadir}/kicad/scripting/*/*.py
%find_lang %{name}
%check
./build/kicad/kicad-cli version
./build/kicad/kicad-cli version --format about
%ctest --exclude-regex 'qa_spice|qa_cli|qa_common|qa_pcbnew'
%ifnarch %{ix86} aarch64
%ctest --tests-regex 'qa_spice|qa_cli|qa_common|qa_pcbnew'
%endif
%ifarch aarch64
%ctest --tests-regex 'qa_cli|qa_common|qa_pcbnew'
# aarch64: https://sourceforge.net/p/ngspice/bugs/622/
%ctest --tests-regex 'qa_spice' || true
%ifnarch %{ix86}
%ctest --tests-regex 'qa_spice|qa_cli|qa_common'
# Occasionally fails
%ctest --repeat until-fail:5 --tests-regex 'qa_pcbnew'
%endif
%ifarch %{ix86}