You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.

164 lines
5.1 KiB

Fix issues with zone filling connectivity locking Two issues found with the locking system used to prevent access to stale connectivity data during the zone fill process: 1) a std::mutex has undefined behavior if you try to use it to guard against access from the same thread. Because of the use of wx event loops (and coroutines) it is entirely possible, and in some situations inevitable, that the same thread will try to redraw the ratsnest in the middle of zone refilling. 2) The mutex was only guarding the ZONE_FILLER::Fill method, but the callers of that method also do connectivity updates as part of the COMMIT::Push. Redrawing the ratsnest after the Fill but before the Push will result in stale connectivity pointers to zone filled areas. Fixed (1) by switching to a trivial spinlock implementation. Spinlocks would generally not be desirable if the contention for the connectivity data crossed thread boundaries, but at the moment I believe it's guaranteed that the reads and writes to connectivity that are guarded by this lock happen from the main UI thread. The writes are also quite rare compared to reads, and reads are generally fast, so I'm not really worried about the UI thread spinning for any real amount of time. Fixed (2) by moving the locking location up to the call sites of ZONE_FILLER::Fill. This issue was quite difficult to reproduce, but I found a fairly reliable way: It only happens (for me) on Windows, MSYS2 build, with wxWidgets 3.0 It also only happens if I restrict PcbNew to use 2 CPU cores. With those conditions, I can reproduce the issue described in #6471 by repeatedly editing a zone properties and changing its net. The crash is especially easy to trigger if you press some keys (such as 'e' for edit) while the progress dialog is displayed. It's easiest to do this in a debug build as the slower KiCad is running, the bigger the window is to trigger this bug. Fixes https://gitlab.com/kicad/code/kicad/-/issues/6471 Fixes https://gitlab.com/kicad/code/kicad/-/issues/7048
5 years ago
  1. /*
  2. * This program source code file is part of KiCad, a free EDA CAD application.
  3. *
  4. * Copyright (C) 2012 Jean-Pierre Charras, jean-pierre.charras@ujf-grenoble.fr
  5. * Copyright (C) 1992-2022 KiCad Developers, see AUTHORS.txt for contributors.
  6. *
  7. * Some code comes from FreePCB.
  8. *
  9. * This program is free software; you can redistribute it and/or
  10. * modify it under the terms of the GNU General Public License
  11. * as published by the Free Software Foundation; either version 2
  12. * of the License, or (at your option) any later version.
  13. *
  14. * This program is distributed in the hope that it will be useful,
  15. * but WITHOUT ANY WARRANTY; without even the implied warranty of
  16. * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
  17. * GNU General Public License for more details.
  18. *
  19. * You should have received a copy of the GNU General Public License
  20. * along with this program; if not, you may find one here:
  21. * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
  22. * or you may search the http://www.gnu.org website for the version 2 license,
  23. * or you may write to the Free Software Foundation, Inc.,
  24. * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
  25. */
  26. #include <kiface_base.h>
  27. #include <confirm.h>
  28. #include <pcb_edit_frame.h>
  29. #include <pcbnew_settings.h>
  30. #include <board_commit.h>
  31. #include <tool/tool_manager.h>
  32. #include <tool/actions.h>
  33. #include <zone.h>
  34. #include <zones.h>
  35. #include <zones_functions_for_undo_redo.h>
  36. #include <connectivity/connectivity_data.h>
  37. void PCB_EDIT_FRAME::Edit_Zone_Params( ZONE* aZone )
  38. {
  39. int dialogResult;
  40. ZONE_SETTINGS zoneInfo = GetZoneSettings();
  41. PICKED_ITEMS_LIST pickedList; // zones for undo/redo command
  42. PICKED_ITEMS_LIST deletedList; // zones that have been deleted when combined
  43. BOARD_COMMIT commit( this );
  44. // Save initial zones configuration, for undo/redo, before adding new zone
  45. // note the net name and the layer can be changed, so we must save all zones
  46. SaveCopyOfZones( pickedList, GetBoard() );
  47. if( aZone->GetIsRuleArea() )
  48. {
  49. // edit a rule area on a copper layer
  50. zoneInfo << *aZone;
  51. dialogResult = InvokeRuleAreaEditor( this, &zoneInfo );
  52. }
  53. else if( IsCopperLayer( aZone->GetFirstLayer() ) )
  54. {
  55. // edit a zone on a copper layer
  56. zoneInfo << *aZone;
  57. dialogResult = InvokeCopperZonesEditor( this, &zoneInfo );
  58. }
  59. else
  60. {
  61. zoneInfo << *aZone;
  62. dialogResult = InvokeNonCopperZonesEditor( this, &zoneInfo );
  63. }
  64. if( dialogResult == wxID_CANCEL )
  65. {
  66. ClearListAndDeleteItems( &deletedList );
  67. ClearListAndDeleteItems( &pickedList );
  68. return;
  69. }
  70. SetZoneSettings( zoneInfo );
  71. OnModify();
  72. if( dialogResult == ZONE_EXPORT_VALUES )
  73. {
  74. UpdateCopyOfZonesList( pickedList, deletedList, GetBoard() );
  75. commit.Stage( pickedList );
  76. commit.Push( _( "Modify zone properties" ) );
  77. pickedList.ClearItemsList(); // s_ItemsListPicker is no more owner of picked items
  78. return;
  79. }
  80. wxBusyCursor dummy;
  81. // Undraw old zone outlines
  82. for( ZONE* zone : GetBoard()->Zones() )
  83. GetCanvas()->GetView()->Update( zone );
  84. zoneInfo.ExportSetting( *aZone );
  85. NETINFO_ITEM* net = GetBoard()->FindNet( zoneInfo.m_NetcodeSelection );
  86. if( net ) // net == NULL should not occur
  87. aZone->SetNetCode( net->GetNetCode() );
  88. UpdateCopyOfZonesList( pickedList, deletedList, GetBoard() );
  89. commit.Stage( pickedList );
  90. commit.Push( _( "Modify zone properties" ), SKIP_CONNECTIVITY );
  91. rebuildConnectivity();
  92. pickedList.ClearItemsList(); // s_ItemsListPicker is no longer owner of picked items
  93. }
  94. bool BOARD::TestZoneIntersection( ZONE* aZone1, ZONE* aZone2 )
  95. {
  96. // see if areas are on same layer
  97. if( !( aZone1->GetLayerSet() & aZone2->GetLayerSet() ).any() )
  98. return false;
  99. SHAPE_POLY_SET* poly1 = aZone1->Outline();
  100. SHAPE_POLY_SET* poly2 = aZone2->Outline();
  101. // test bounding rects
  102. BOX2I b1 = poly1->BBox();
  103. BOX2I b2 = poly2->BBox();
  104. if( ! b1.Intersects( b2 ) )
  105. return false;
  106. // Now test for intersecting segments
  107. for( auto segIterator1 = poly1->IterateSegmentsWithHoles(); segIterator1; segIterator1++ )
  108. {
  109. // Build segment
  110. SEG firstSegment = *segIterator1;
  111. for( auto segIterator2 = poly2->IterateSegmentsWithHoles(); segIterator2; segIterator2++ )
  112. {
  113. // Build second segment
  114. SEG secondSegment = *segIterator2;
  115. // Check whether the two segments built collide
  116. if( firstSegment.Collide( secondSegment, 0 ) )
  117. return true;
  118. }
  119. }
  120. // If a contour is inside another contour, no segments intersects, but the zones
  121. // can be combined if a corner is inside an outline (only one corner is enough)
  122. for( auto iter = poly2->IterateWithHoles(); iter; iter++ )
  123. {
  124. if( poly1->Contains( *iter ) )
  125. return true;
  126. }
  127. for( auto iter = poly1->IterateWithHoles(); iter; iter++ )
  128. {
  129. if( poly2->Contains( *iter ) )
  130. return true;
  131. }
  132. return false;
  133. }