alecthomas / entityx

EntityX - A fast, type-safe C++ Entity-Component system
MIT License
2.21k stars 295 forks source link

Acces violations when more than 1024 entities are created (compile_time_branch) #176

Closed roig closed 7 years ago

roig commented 7 years ago

I'm experiencing some nasty access violations to my components member variables maybe due to object memory data corruption or overflow. I'm trying to send you the minimal code to reproduce it but it's hard, I think it only happens when more than 1024 entities with components are created.

I tried the ColumnStorage and ContiguousStorage and I have similar acces violations on both with more than 1024 entities.

Have you any test or something that check what happens after the INITIAL_CAPACITY = 1024 capacity is resized? I will post again with more info, but maybe there is a bug when resize happens.

alecthomas commented 7 years ago

Are you maintaining references to components across iterations?

roig commented 7 years ago

No, I'm struggling to get a minimal code that reproduce it but here you have something :

        std::vector<entity> entities_inserted;
        for (auto i = 0; i < 1025; i++) {
            auto e = w.create();
            e.assign<com::transform>();
            entities_inserted.push_back(e);
        }

        log::log_msg("DONE 0");

        for (auto &e : entities_inserted) {
            log::log_msg("one");
            auto tr = e.component<com::transform>();
            tr->unmark_dirty();
            tr->translate({0,100,0});
        }

If I create 1024 it doesn't crash, if I create 1025 it crashes after tr->unmark_dirty(); on the first iteration There is an std::set inside the com::transform component, that have size = 0 BUT gets corrupted somewhere and the begin and end iterators are not equals, then when it tries to acces the first element it crashes.

alecthomas commented 7 years ago

If you look in the examples directory you'll see an example that creates 20-30k entities. Can you try that? Compile instructions are at the top. It runs give on my machine.

roig commented 7 years ago

Yes, I tried the example and it works very well.

But can you try this example ? In this code the std::set memory, gets corrupted.

#include "entityx/entityx.hh"
#include <set>
#include <vector>
#include <iostream>

class test_component {
public:
    std::set<int> emptyset;
    void test_set() {
        if (emptyset.begin() != emptyset.end()) {
            std::cout<< "SOMETHING IS VERY BAD, AND CORRUPTED"<<std::endl;
        }

        for (auto& item: emptyset) {
            (void)item;
            std::cout<< "THAT'S NOT POSSIBLE!!! >_<"<< std::endl;
        }
    }
};

using cs = entityx::Components<test_component>;
using eworld = entityx::EntityX<cs, entityx::ContiguousStorage<cs>, entityx::OBSERVABLE>;

int main() {
    eworld ew;

    std::vector<eworld::Entity> entities_inserted;

    // insert 1025 and it will corrupt the memory and crash,
    // insert 1024 and it will run ok
    for (auto i = 0; i < 1025; i++) {
        auto e = ew.create();
        entities_inserted.push_back(e);
        e.assign<test_component>();
    }

    std::cout<< "INSERTION DONE"<< std::endl;

    for (auto &e : entities_inserted) {
        auto tc = e.component<test_component>();
        tc->test_set();
    }

    std::cout<< "WORKS OK"<< std::endl;
}

My output is:

INSERTION DONE
SOMETHING IS VERY BAD, AND CORRUPTED
THAT'S NOT POSSIBLE!!! >_<
Segmentation fault
alecthomas commented 7 years ago

Okay, interesting. I'll take a look. Does the example code work for you?

roig commented 7 years ago

Yes, the example works very well and in fullscreen glory :dancer: . Are you getting the Segmentation fault on my code too?

alecthomas commented 7 years ago

I've found the bug: neither storage implementation calls copy constructors; only a bitwise copy is performed. This doesn't occur in the example because the components don't contain non-PoD objects.

roig commented 7 years ago

Ok, that sounds great! Can you fix it ? I will try the fix with my project, and btw this branch works very well (apart from this bug). Maybe you can think about releasing it.

Thank you!

alecthomas commented 7 years ago

Ironically, I've been holding off on releasing it because I plan on refactoring how storage backends work, so fixing this will motivate me to do that. I'm not sure how long it will be though, as I'm travelling at the moment.

roig commented 7 years ago

No problem, as a workaround I think that modifying the INITIAL_CAPACITY to a greater value will work till you finish to refactor the storage backend. Thank you for this great tool !!

L3nn0x commented 7 years ago

I am also using this branch in my project, and I'll need way more than 1024 items once I finish it. Thanks for the workaround and the warning about corruption! It'll save me a ton of time once I start ramping up those entities.

alecthomas commented 7 years ago

This should be fixed now. Give it a whirl and let me know.