From 20111ce08d83b0498750991e6b866adf0de00704 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Mon, 3 Mar 2025 01:05:04 +0100 Subject: [PATCH] Fixed issue #1993 ('with_area' modifies result of 'join') Root cause was a held layout lock inside the TilingProcessor. As that object was not always released inside the Ruby DRC script, the lock was held sometimes longer than needed and messed with the hierarchy system. --- src/db/db/dbOriginalLayerEdgePairs.cc | 2 +- src/db/db/dbOriginalLayerEdges.cc | 2 +- src/db/db/dbOriginalLayerRegion.cc | 6 +----- src/db/db/dbOriginalLayerTexts.cc | 2 +- src/db/db/dbRecursiveShapeIterator.cc | 7 +++++++ src/db/db/dbRecursiveShapeIterator.h | 10 ++++++++++ src/db/db/dbTilingProcessor.cc | 2 +- src/db/unit_tests/dbTilingProcessorTests.cc | 2 ++ src/drc/drc/built-in-macros/_drc_layer.rb | 2 ++ 9 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/db/db/dbOriginalLayerEdgePairs.cc b/src/db/db/dbOriginalLayerEdgePairs.cc index cf9b99c27..3544512fe 100644 --- a/src/db/db/dbOriginalLayerEdgePairs.cc +++ b/src/db/db/dbOriginalLayerEdgePairs.cc @@ -181,7 +181,7 @@ OriginalLayerEdgePairs::begin_iter () const bool OriginalLayerEdgePairs::empty () const { - return m_iter.at_end (); + return m_iter.at_end_no_lock (); } const db::EdgePair * diff --git a/src/db/db/dbOriginalLayerEdges.cc b/src/db/db/dbOriginalLayerEdges.cc index 1cb19d028..da181c17d 100644 --- a/src/db/db/dbOriginalLayerEdges.cc +++ b/src/db/db/dbOriginalLayerEdges.cc @@ -218,7 +218,7 @@ OriginalLayerEdges::begin_merged_iter () const bool OriginalLayerEdges::empty () const { - return m_iter.at_end (); + return m_iter.at_end_no_lock (); } bool diff --git a/src/db/db/dbOriginalLayerRegion.cc b/src/db/db/dbOriginalLayerRegion.cc index d089f4001..d5ba1cf13 100644 --- a/src/db/db/dbOriginalLayerRegion.cc +++ b/src/db/db/dbOriginalLayerRegion.cc @@ -330,11 +330,7 @@ OriginalLayerRegion::begin_merged_iter () const bool OriginalLayerRegion::empty () const { - // NOTE: we should to make sure the iterator isn't validated as this would spoil the usability or OriginalLayerRegion upon - // layout changes - db::RecursiveShapeIterator iter = m_iter; - - return iter.at_end (); + return m_iter.at_end_no_lock (); } bool diff --git a/src/db/db/dbOriginalLayerTexts.cc b/src/db/db/dbOriginalLayerTexts.cc index 6555eb4dc..9d8622f43 100644 --- a/src/db/db/dbOriginalLayerTexts.cc +++ b/src/db/db/dbOriginalLayerTexts.cc @@ -181,7 +181,7 @@ OriginalLayerTexts::begin_iter () const bool OriginalLayerTexts::empty () const { - return m_iter.at_end (); + return m_iter.at_end_no_lock (); } const db::Text * diff --git a/src/db/db/dbRecursiveShapeIterator.cc b/src/db/db/dbRecursiveShapeIterator.cc index fda051d39..c92cd4bc4 100644 --- a/src/db/db/dbRecursiveShapeIterator.cc +++ b/src/db/db/dbRecursiveShapeIterator.cc @@ -598,6 +598,13 @@ RecursiveShapeIterator::at_end () const return m_shape.at_end () || is_inactive (); } +bool +RecursiveShapeIterator::at_end_no_lock () const +{ + RecursiveShapeIterator copy (*this); + return copy.at_end (); +} + std::vector RecursiveShapeIterator::path () const { diff --git a/src/db/db/dbRecursiveShapeIterator.h b/src/db/db/dbRecursiveShapeIterator.h index ae337a29e..26ee32b7e 100644 --- a/src/db/db/dbRecursiveShapeIterator.h +++ b/src/db/db/dbRecursiveShapeIterator.h @@ -709,6 +709,16 @@ public: */ bool at_end () const; + /** + * @brief End of iterator predicate + * + * Returns true, if the iterator is at the end of the sequence + * + * This version does not lock the layout and can be used after initialization + * to detect empty sequences. + */ + bool at_end_no_lock () const; + /** * @brief Gets the translated property ID * diff --git a/src/db/db/dbTilingProcessor.cc b/src/db/db/dbTilingProcessor.cc index b25ce543a..131c3bb4f 100644 --- a/src/db/db/dbTilingProcessor.cc +++ b/src/db/db/dbTilingProcessor.cc @@ -876,7 +876,7 @@ TilingProcessor::execute (const std::string &desc) if (tot_box.empty ()) { for (std::vector::const_iterator i = m_inputs.begin (); i != m_inputs.end (); ++i) { - if (! i->iter.at_end ()) { + if (! i->iter.at_end_no_lock ()) { if (scale_to_dbu ()) { double dbu_value = i->iter.layout () ? i->iter.layout ()->dbu () : dbu (); tot_box += i->iter.bbox ().transformed (db::CplxTrans (dbu_value) * db::CplxTrans (i->trans)); diff --git a/src/db/unit_tests/dbTilingProcessorTests.cc b/src/db/unit_tests/dbTilingProcessorTests.cc index e31a885e5..18a741fce 100644 --- a/src/db/unit_tests/dbTilingProcessorTests.cc +++ b/src/db/unit_tests/dbTilingProcessorTests.cc @@ -148,7 +148,9 @@ TEST(2) tp.queue ("_output(o1, _tile ? (i1 & i2 & _tile) : (i1 & i2), false)"); tp.queue ("!_tile && _output(o2, i1.outside(i2), false)"); tp.queue ("_tile && _output(o3, _tile, false)"); + EXPECT_EQ (ly.under_construction (), false); tp.execute ("test"); + EXPECT_EQ (ly.under_construction (), false); EXPECT_EQ (to_s (ly, top, o1), "box (60,10;70,20);box (10,10;30,30)"); EXPECT_EQ (to_s (ly, top, o2), "box (50,40;80,70)"); diff --git a/src/drc/drc/built-in-macros/_drc_layer.rb b/src/drc/drc/built-in-macros/_drc_layer.rb index c384d82c6..25f47c22d 100644 --- a/src/drc/drc/built-in-macros/_drc_layer.rb +++ b/src/drc/drc/built-in-macros/_drc_layer.rb @@ -4624,6 +4624,8 @@ TP_SCRIPT res end + tp._destroy + DRCLayer::new(@engine, res) end