From 04885b5ba73b2ffc12258d6df2d2d98fcdcba3fe Mon Sep 17 00:00:00 2001
From: Kenneth Moreland <kmorel@sandia.gov>
Date: Wed, 13 May 2020 13:52:39 -0600
Subject: [PATCH] Fix thread safety issue in vtkUnstructuredGrid::GetCell

The vtkUnstructuredGrid::GetCell method that takes a vtkGenericCell is
supposed to be thread safe. However, there was an issue where GetCell
called a method that was unsafe under certain conditions.

The problem was thta GetCell was calling the version of
vtkCellArray::GetCellAtId that returned the pointer to the internal
array of indices. That was thread safe if vtkCellArray happened to use a
simple array of type vtkIdType to store the indices. However, if the
array was of a different type, it copied the values to an array stored
in vtkCellArray and a pointer to that is returned. This is very much not
thread safe.

Fixed the problem by using a different version of GetCellAtId that takes
in the vtkIdList of the generic cell. Since the connectivity has to be
copied to that array anyway, there is no real performance penalty for
this.
---
 Common/DataModel/vtkUnstructuredGrid.cxx        |  8 +-------
 .../release/dev/ug-getcell-thread-safety.md     | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/release/dev/ug-getcell-thread-safety.md

diff --git a/Common/DataModel/vtkUnstructuredGrid.cxx b/Common/DataModel/vtkUnstructuredGrid.cxx
index 86662450c43..4f470a7d72e 100644
--- a/Common/DataModel/vtkUnstructuredGrid.cxx
+++ b/Common/DataModel/vtkUnstructuredGrid.cxx
@@ -994,13 +994,7 @@ void vtkUnstructuredGrid::GetCell(vtkIdType cellId, vtkGenericCell* cell)
   int cellType = static_cast<int>(this->Types->GetValue(cellId));
   cell->SetCellType(cellType);
 
-  vtkIdType numPts;
-  const vtkIdType* pts;
-  this->Connectivity->GetCellAtId(cellId, numPts, pts);
-
-  cell->PointIds->SetNumberOfIds(numPts);
-
-  std::copy(pts, pts + numPts, cell->PointIds->GetPointer(0));
+  this->Connectivity->GetCellAtId(cellId, cell->PointIds);
   this->Points->GetPoints(cell->PointIds, cell->Points);
 
   // Explicit face representation
diff --git a/Documentation/release/dev/ug-getcell-thread-safety.md b/Documentation/release/dev/ug-getcell-thread-safety.md
new file mode 100644
index 00000000000..3ae4c694c2f
--- /dev/null
+++ b/Documentation/release/dev/ug-getcell-thread-safety.md
@@ -0,0 +1,17 @@
+## Fix thread safety issue in vtkUnstructuredGrid::GetCell
+
+The `vtkUnstructuredGrid::GetCell` method that takes a `vtkGenericCell` is
+supposed to be thread safe. However, there was an issue where `GetCell`
+called a method that was unsafe under certain conditions.
+The problem was thta GetCell was calling the version of
+`vtkCellArray::GetCellAtId` that returned the pointer to the internal
+array of indices. That was thread safe if `vtkCellArray` happened to use a
+simple array of type `vtkIdType` to store the indices. However, if the
+array was of a different type, it copied the values to an array stored
+in `vtkCellArray` and a pointer to that is returned. This is very much not
+thread safe.
+
+The problem is fixed by using a different version of `GetCellAtId` that
+takes in the `vtkIdList` of the generic cell. Since the connectivity has to
+be copied to that array anyway, there is no real performance penalty for
+this.
-- 
GitLab