Fix crash bug, improve Unit Test

Crash bug from improperly implemented std::lock_guard for multi-threaded
usage of forMatchingSignatures fixed.

diff --git a/src/EC/Manager.hpp b/src/EC/Manager.hpp
index 378ca7a8956654bfd0e8d6f2922f15078b4bea84..15ff27c0b60187ca0e96af9e8edabfc6372062c9 100644 (file)
--- a/src/EC/Manager.hpp
+++ b/src/EC/Manager.hpp
@@ -24,6 +24,10 @@
 #include <queue>
 #include <mutex>
 
+#ifndef NDEBUG
+  #include <iostream>
+#endif
+
 #include "Meta/Combine.hpp"
 #include "Meta/Matching.hpp"
 #include "Bitset.hpp"
@@ -966,8 +970,6 @@ namespace EC
             return forMatchingFunctions.erase(index) == 1;
         }
 
-    public:
-
         /*!
             \brief Call multiple functions with mulitple signatures on all
                 living entities.
@@ -1074,8 +1076,10 @@ namespace EC
                                     & std::get<BitsetType>(entities[eid]))
                                         == signatureBitsets[i])
                                 {
-                                    std::lock_guard<std::mutex>{sigsMutexes[i]};
+                                    std::lock_guard<std::mutex> guard(
+                                        sigsMutexes[i]);
                                     multiMatchingEntities[i].push_back(eid);
+
                                 }
                             }
                         }
@@ -1101,19 +1105,20 @@ namespace EC
                         ForMatchingSignatureHelper<> >;
                 using Index = EC::Meta::IndexOf<decltype(signature),
                     SigList>;
+                constexpr std::size_t index = Index{};
                 if(threadCount <= 1)
                 {
-                    for(auto iter = multiMatchingEntities[Index{}].begin();
-                        iter != multiMatchingEntities[Index{}].end(); ++iter)
+                    for(auto iter = multiMatchingEntities[index].begin();
+                        iter != multiMatchingEntities[index].end(); ++iter)
                     {
                         Helper::call(*iter, *this,
-                            std::get<Index{}>(funcTuple));
+                            std::get<index>(funcTuple));
                     }
                 }
                 else
                 {
                     std::vector<std::thread> threads(threadCount);
-                    std::size_t s = multiMatchingEntities[Index{}].size()
+                    std::size_t s = multiMatchingEntities[index].size()
                         / threadCount;
                     for(std::size_t i = 0; i < threadCount; ++i)
                     {
@@ -1121,7 +1126,7 @@ namespace EC
                         std::size_t end;
                         if(i == threadCount - 1)
                         {
-                            end = multiMatchingEntities[Index{}].size();
+                            end = multiMatchingEntities[index].size();
                         }
                         else
                         {
@@ -1131,10 +1136,10 @@ namespace EC
                         [this, &multiMatchingEntities, &funcTuple]
                         (std::size_t begin, std::size_t end)
                         {
-                            for(std::size_t i = begin; i < end; ++i)
+                            for(std::size_t j = begin; j < end; ++j)
                             {
-                                Helper::call(multiMatchingEntities[Index{}][i],
-                                    *this, std::get<Index{}>(funcTuple));
+                                Helper::call(multiMatchingEntities[index][j],
+                                    *this, std::get<index>(funcTuple));
                             }
                         }, begin, end);
                     }

diff --git a/src/test/ECTest.cpp b/src/test/ECTest.cpp
index 8100264ed0b7a73040b6ac8ece679cc84688b5ac..2cf3b1106333deabf6015b58420a0719225ab080 100644 (file)
--- a/src/test/ECTest.cpp
+++ b/src/test/ECTest.cpp
@@ -4,6 +4,8 @@
 #include <iostream>
 #include <tuple>
 #include <memory>
+#include <unordered_map>
+#include <mutex>
 
 #include <EC/Meta/Meta.hpp>
 #include <EC/EC.hpp>
@@ -586,22 +588,35 @@ TEST(EC, ForMatchingSignatures)
 {
     EC::Manager<ListComponentsAll, ListTagsAll> manager;
 
-    auto e = {
+    std::size_t e[] = {
+        manager.addEntity(),
+        manager.addEntity(),
+        manager.addEntity(),
         manager.addEntity(),
         manager.addEntity(),
         manager.addEntity(),
         manager.addEntity()
     };
 
+    auto& first = e[0];
+    auto& last = e[6];
+
     for(auto id : e)
     {
-        manager.addComponent<C0>(id);
-        manager.addComponent<C1>(id);
-        manager.addTag<T0>(id);
+        if(id != first && id != last)
+        {
+            manager.addComponent<C0>(id);
+            manager.addComponent<C1>(id);
+            manager.addTag<T0>(id);
 
-        auto& c1 = manager.getEntityData<C1>(id);
-        c1.vx = 0;
-        c1.vy = 0;
+            auto& c1 = manager.getEntityData<C1>(id);
+            c1.vx = 0;
+            c1.vy = 0;
+        }
+        else
+        {
+            manager.addComponent<C0>(id);
+        }
     }
 
     using namespace EC::Meta;
@@ -630,28 +645,85 @@ TEST(EC, ForMatchingSignatures)
 
     for(auto id : e)
     {
-        EXPECT_EQ(2, manager.getEntityData<C0>(id).x);
-        EXPECT_EQ(2, manager.getEntityData<C0>(id).y);
-        EXPECT_EQ(1, manager.getEntityData<C1>(id).vx);
-        EXPECT_EQ(1, manager.getEntityData<C1>(id).vy);
+        if(id != first && id != last)
+        {
+            EXPECT_EQ(2, manager.getEntityData<C0>(id).x);
+            EXPECT_EQ(2, manager.getEntityData<C0>(id).y);
+            EXPECT_EQ(1, manager.getEntityData<C1>(id).vx);
+            EXPECT_EQ(1, manager.getEntityData<C1>(id).vy);
+        }
+        else
+        {
+            EXPECT_EQ(1, manager.getEntityData<C0>(id).x);
+            EXPECT_EQ(1, manager.getEntityData<C0>(id).y);
+        }
     }
 
+    {
+    std::unordered_map<std::size_t,int> cx;
+    std::unordered_map<std::size_t,int> cy;
+    std::unordered_map<std::size_t,int> c0x;
+    std::unordered_map<std::size_t,int> c0y;
+    std::unordered_map<std::size_t,int> c1vx;
+    std::unordered_map<std::size_t,int> c1vy;
+    std::mutex cxM;
+    std::mutex cyM;
+    std::mutex c0xM;
+    std::mutex c0yM;
+    std::mutex c1vxM;
+    std::mutex c1vyM;
+
     manager.forMatchingSignatures<
         TypeList<TypeList<C0>, TypeList<C0, C1> >
     >
     (
         std::make_tuple(
-        [] (std::size_t eid, C0& c) {
-            EXPECT_EQ(2, c.x);
-            EXPECT_EQ(2, c.y);
-            c.x = 5;
-            c.y = 7;
+        [&first, &last, &cx, &cy, &cxM, &cyM] (std::size_t eid, C0& c) {
+            if(eid != first && eid != last)
+            {
+                {
+                    std::lock_guard<std::mutex> guard(cxM);
+                    cx.insert(std::make_pair(eid, c.x));
+                }
+                {
+                    std::lock_guard<std::mutex> guard(cyM);
+                    cy.insert(std::make_pair(eid, c.y));
+                }
+                c.x = 5;
+                c.y = 7;
+            }
+            else
+            {
+                {
+                    std::lock_guard<std::mutex> guard(cxM);
+                    cx.insert(std::make_pair(eid, c.x));
+                }
+                {
+                    std::lock_guard<std::mutex> guard(cyM);
+                    cy.insert(std::make_pair(eid, c.y));
+                }
+                c.x = 11;
+                c.y = 13;
+            }
         },
-        [] (std::size_t eid, C0& c0, C1& c1) {
-            EXPECT_EQ(5, c0.x);
-            EXPECT_EQ(7, c0.y);
-            EXPECT_EQ(1, c1.vx);
-            EXPECT_EQ(1, c1.vy);
+        [&c0x, &c0y, &c1vx, &c1vy, &c0xM, &c0yM, &c1vxM, &c1vyM]
+        (std::size_t eid, C0& c0, C1& c1) {
+            {
+                std::lock_guard<std::mutex> guard(c0xM);
+                c0x.insert(std::make_pair(eid, c0.x));
+            }
+            {
+                std::lock_guard<std::mutex> guard(c0yM);
+                c0y.insert(std::make_pair(eid, c0.y));
+            }
+            {
+                std::lock_guard<std::mutex> guard(c1vxM);
+                c1vx.insert(std::make_pair(eid, c1.vx));
+            }
+            {
+                std::lock_guard<std::mutex> guard(c1vyM);
+                c1vy.insert(std::make_pair(eid, c1.vy));
+            }
 
             c1.vx += c0.x;
             c1.vy += c0.y;
@@ -661,13 +733,61 @@ TEST(EC, ForMatchingSignatures)
         }),
         3
     );
+    
+    for(auto iter = cx.begin(); iter != cx.end(); ++iter)
+    {
+        if(iter->first != first && iter->first != last)
+        {
+            EXPECT_EQ(2, iter->second);
+        }
+        else
+        {
+            EXPECT_EQ(1, iter->second);
+        }
+    }
+    for(auto iter = cy.begin(); iter != cy.end(); ++iter)
+    {
+        if(iter->first != first && iter->first != last)
+        {
+            EXPECT_EQ(2, iter->second);
+        }
+        else
+        {
+            EXPECT_EQ(1, iter->second);
+        }
+    }
+    for(auto iter = c0x.begin(); iter != c0x.end(); ++iter)
+    {
+        EXPECT_EQ(5, iter->second);
+    }
+    for(auto iter = c0y.begin(); iter != c0y.end(); ++iter)
+    {
+        EXPECT_EQ(7, iter->second);
+    }
+    for(auto iter = c1vx.begin(); iter != c1vx.end(); ++iter)
+    {
+        EXPECT_EQ(1, iter->second);
+    }
+    for(auto iter = c1vy.begin(); iter != c1vy.end(); ++iter)
+    {
+        EXPECT_EQ(1, iter->second);
+    }
+    }
 
     for(auto eid : e)
     {
-        EXPECT_EQ(1, manager.getEntityData<C0>(eid).x);
-        EXPECT_EQ(2, manager.getEntityData<C0>(eid).y);
-        EXPECT_EQ(6, manager.getEntityData<C1>(eid).vx);
-        EXPECT_EQ(8, manager.getEntityData<C1>(eid).vy);
+        if(eid != first && eid != last)
+        {
+            EXPECT_EQ(1, manager.getEntityData<C0>(eid).x);
+            EXPECT_EQ(2, manager.getEntityData<C0>(eid).y);
+            EXPECT_EQ(6, manager.getEntityData<C1>(eid).vx);
+            EXPECT_EQ(8, manager.getEntityData<C1>(eid).vy);
+        }
+        else
+        {
+            EXPECT_EQ(11, manager.getEntityData<C0>(eid).x);
+            EXPECT_EQ(13, manager.getEntityData<C0>(eid).y);
+        }
     }
 }