commit 6ae43c40de8cc35af3b3ba401fa6504ff782cd86
author Matthew Bentham <matthew.bentham@arm.com> Mon Jan 10 13:34:12 2022 +0000
committer TeresaARM <teresa.charlinreyes@arm.com> Wed Jan 12 20:09:14 2022 +0000
tree 1aff6d2280752fae0f2b5357be1588c1720df432
parent 3ea0107ce9971cea47ac6e318cc9affbd9b6a989

Fix thread safety issues in TimelineDecoder and associated unit tests

Enforce serialized access to TimelineDecoder::m_Model by removing public
access funtion and replacing with an 'Apply' method taking a lambda and
uses a std::lock.

Use the new lambda when invoking callbacks.

Change-Id: I6ea2fbca990736f3be63e80897f175421f19f0c1
Signed-off-by: Matthew Bentham <matthew.bentham@arm.com>

profiling/server/src/timelineDecoder/tests/TimelineTests.cpp

diff --git a/profiling/server/src/timelineDecoder/tests/TimelineTests.cpp b/profiling/server/src/timelineDecoder/tests/TimelineTests.cpp
index f09848f..82c16fe 100644
--- a/profiling/server/src/timelineDecoder/tests/TimelineTests.cpp
+++ b/profiling/server/src/timelineDecoder/tests/TimelineTests.cpp
@@ -154,8 +154,6 @@
     arm::pipe::PacketVersionResolver packetVersionResolver;
 
     arm::pipe::TimelineDecoder timelineDecoder;
-    const arm::pipe::TimelineDecoder::Model& model = timelineDecoder.GetModel();
-
 
     arm::pipe::TimelineCaptureCommandHandler timelineCaptureCommandHandler(
         1, 1, packetVersionResolver.ResolvePacketVersion(1, 1).GetEncodedValue(), timelineDecoder,
@@ -238,25 +236,27 @@
                                            timelineCaptureCommandHandler);
     }
 
-    for (unsigned long i = 0; i < 10; ++i)
-    {
-        CHECK(model.m_Entities[i].m_Guid == entityGuid);
+    timelineDecoder.ApplyToModel([&](const arm::pipe::TimelineDecoder::Model& model){
+        for (unsigned long i = 0; i < 10; ++i)
+        {
+            CHECK(model.m_Entities[i].m_Guid == entityGuid);
 
-        CHECK(model.m_EventClasses[i].m_Guid == eventClassGuid);
+            CHECK(model.m_EventClasses[i].m_Guid == eventClassGuid);
 
-        CHECK(model.m_Events[i].m_TimeStamp == timestamp);
-        CHECK(model.m_Events[i].m_ThreadId == uint64ThreadId);
-        CHECK(model.m_Events[i].m_Guid == eventGuid);
+            CHECK(model.m_Events[i].m_TimeStamp == timestamp);
+            CHECK(model.m_Events[i].m_ThreadId == uint64ThreadId);
+            CHECK(model.m_Events[i].m_Guid == eventGuid);
 
-        CHECK(model.m_Labels[i].m_Guid == labelGuid);
-        CHECK(model.m_Labels[i].m_Name == labelName);
+            CHECK(model.m_Labels[i].m_Guid == labelGuid);
+            CHECK(model.m_Labels[i].m_Name == labelName);
 
-        CHECK(model.m_Relationships[i].m_RelationshipType ==
-            arm::pipe::ITimelineDecoder::RelationshipType::DataLink);
-        CHECK(model.m_Relationships[i].m_Guid == relationshipGuid);
-        CHECK(model.m_Relationships[i].m_HeadGuid == headGuid);
-        CHECK(model.m_Relationships[i].m_TailGuid == tailGuid);
-    }
+            CHECK(model.m_Relationships[i].m_RelationshipType ==
+                arm::pipe::ITimelineDecoder::RelationshipType::DataLink);
+            CHECK(model.m_Relationships[i].m_Guid == relationshipGuid);
+            CHECK(model.m_Relationships[i].m_HeadGuid == headGuid);
+            CHECK(model.m_Relationships[i].m_TailGuid == tailGuid);
+        }
+    });
 }
 
 TEST_CASE("TimelineCaptureTestMultipleStringsInBuffer")
@@ -270,7 +270,6 @@
     arm::pipe::PacketVersionResolver packetVersionResolver;
 
     arm::pipe::TimelineDecoder timelineDecoder;
-    const arm::pipe::TimelineDecoder::Model& model = timelineDecoder.GetModel();
 
     arm::pipe::TimelineCaptureCommandHandler timelineCaptureCommandHandler(
         1, 1, packetVersionResolver.ResolvePacketVersion(1, 1).GetEncodedValue(), timelineDecoder,
@@ -343,30 +342,32 @@
     SendTimelinePacketToCommandHandler(bufferManager.GetReadableBuffer()->GetReadableData(),
                                        timelineCaptureCommandHandler);
 
-    for ( unsigned long i = 0; i < 9; ++i )
-    {
-        CHECK(model.m_Entities[i].m_Guid == entityGuid);
+    timelineDecoder.ApplyToModel([&](const arm::pipe::TimelineDecoder::Model& model){
+        for ( unsigned long i = 0; i < 9; ++i )
+        {
+            CHECK(model.m_Entities[i].m_Guid == entityGuid);
 
-        CHECK(model.m_EventClasses[i].m_Guid == eventClassGuid);
+            CHECK(model.m_EventClasses[i].m_Guid == eventClassGuid);
 
-        CHECK(model.m_Labels[i].m_Guid == labelGuid);
+            CHECK(model.m_Labels[i].m_Guid == labelGuid);
 
-        CHECK(model.m_Events[i].m_TimeStamp == timestamp);
-        CHECK(model.m_Events[i].m_ThreadId == uint64ThreadId);
-        CHECK(model.m_Events[i].m_Guid == eventGuid);
+            CHECK(model.m_Events[i].m_TimeStamp == timestamp);
+            CHECK(model.m_Events[i].m_ThreadId == uint64ThreadId);
+            CHECK(model.m_Events[i].m_Guid == eventGuid);
 
-        CHECK(model.m_Relationships[i].m_RelationshipType ==
-            arm::pipe::ITimelineDecoder::RelationshipType::DataLink);
-        CHECK(model.m_Relationships[i].m_Guid == relationshipGuid);
-        CHECK(model.m_Relationships[i].m_HeadGuid == headGuid);
-        CHECK(model.m_Relationships[i].m_TailGuid == tailGuid);
-    }
-    for ( unsigned long i = 0; i < 9; i += 3 )
-    {
-        CHECK(model.m_Labels[i].m_Name == labelName);
-        CHECK(model.m_Labels[i+1].m_Name == labelName2);
-        CHECK(model.m_Labels[i+2].m_Name == labelName3);
-    }
+            CHECK(model.m_Relationships[i].m_RelationshipType ==
+                arm::pipe::ITimelineDecoder::RelationshipType::DataLink);
+            CHECK(model.m_Relationships[i].m_Guid == relationshipGuid);
+            CHECK(model.m_Relationships[i].m_HeadGuid == headGuid);
+            CHECK(model.m_Relationships[i].m_TailGuid == tailGuid);
+        }
+        for ( unsigned long i = 0; i < 9; i += 3 )
+        {
+            CHECK(model.m_Labels[i].m_Name == labelName);
+            CHECK(model.m_Labels[i+1].m_Name == labelName2);
+            CHECK(model.m_Labels[i+2].m_Name == labelName3);
+        }
+    });
 }
 
 }