Access violation when using registry.clear<Component>()

[JohnMrziglod]

Hi there,

I am using entt 3.13.1 from vcpkg with MSVC 19.37.32825.0 and I get sometimes an access violation when using .clear():

registry.clear<Component>();

Unfortunately, it only happens sometimes and I couldn't reproduce it yet with a minimal example. It runs thousands of times without a problem and suddenly crashes.

According to debug info, it fails in sparse_set.hpp, line 258:

packed[static_cast<size_type>(entt)] = packed.back();

I use the vanilla entt types and pools without any specific customisation, etc.

I am trying to reduce it down but it is quite difficult. If you have any ideas or tips, I would be very happy :-)

[skypjack]

Yeah, a repro would help a lot, you know.
Judging by the error, it looks as if you were using a moved from storage but it's hard to confirm it.
Do you happen to move the registry in your codebase? You can try to redefine its move ctor/op and assert in there to start with.
Can you provide more details on how you create and use these storage instances? It could also happen because of a dangling pointer due to a concurrent access to the same storage but it would be gross and easy to spot probably.
Let me know if you have more details to share. 👍

[JohnMrziglod]

Thank you for your response! I don't think I move the registry anywhere but I will check later by using your assert method.

I create and clear the storage like this:

registry.clear<Renderable>();

std::vector<entt::entity> layer_entities;
for (auto entity: layer_entities)
{
  if (not registry.all_of<Position, Drawable>(entity))
    continue;

  registry.emplace<Renderable>(entity, ...);
}

Then I use it via registry.view<...>().each(...) at many places in my codebase, e.g.

registry.view<Renderable, Drawable>().each(
[](auto entity, auto &renderable, auto &drawable){
  // do something ...
  // but neither create or remove any components from the registry
});

[skypjack]

Side note:

I create and clear the storage like this:

I suggest creating a view<entt::get_t<Position, Drawable>> and use it with your if. It's much faster because it avoids the storage lookup otherwise performed by the registry. Similarly:

registry.emplace<Renderable>(entity, ...);

Get the storage once and use it again and again as in storage.emplace(entity). Same reason as above. In a tight loop, it can help. 👍
As for your problem, do you happen to use a registry from multiple threads? Consider that pool creation isn't thread safe and could quickly lead to UB if you don't create pools in a warm-up tick or function.
If it's not the case, well, I'm trying to think aloud and guess since I don't have a repro. I hope it helps. 👍

[JohnMrziglod]

I suggest creating a view<entt::get_t<Position, Drawable>> and use it with your if. It's much faster because it avoids the storage > lookup otherwise performed by the registry.

Oh, I didn't know that. So i could write this?

auto storage = registry.view<entt::get_t<Position, Drawable>>();
for (auto entity: entities) {
    if (not storage.contains(entity))
        continue;
    // ...
}

[JohnMrziglod]

As for your problem, do you happen to use a registry from multiple threads? Consider that pool creation isn't thread safe and
could quickly lead to UB if you don't create pools in a warm-up tick or function.
If it's not the case, well, I'm trying to think aloud and guess since I don't have a repro. I hope it helps. 👍

Just to make sure, I understand you correctly. I am not allowed to (or rather I shouldn't) do things like this:

// Create a new component while iterating over the same component pool:
auto view = registry.view<Position>();
std::for_each(std::execution::par_unseq, view.begin(), view.end(),
  [&registry](auto entity, auto &position){
    // DON'T DO THIS
    auto new_entity = registry.create();
    registry.emplace<Renderable>(new_entity);
  });

[skypjack]

Oh, I didn't know that. So i could write this?

auto storage = registry.view<entt::get_t<Position, Drawable>>();
for (auto entity: entities) {
    if (not storage.contains(entity))
        continue;
    // ...
}

Yeah, exactly. The view doesn't have to lookup the storage, so the check is faster overall and the lookup costs don't sum up.

Just to make sure, I understand you correctly. I am not allowed to (or rather I shouldn't) do things like this:

Yes and no.
Of course, creating instances of the same component type from different threads is risky. It's as if you were pushing at the end of a vector from different threads. Bad things will happen for sure sooner or later. 🙂
What I mean is this though:

thread 1: registry.emplace<T>();
thread 2: registry.emplace<U>();

This is ok in general but only if the pools already exist, so only after the first time.
If you're in doubt, I suggest running a single threaded warmup tick or forcing pool creation on the main thread before spawning other threads. All you have to do is registry.storage<T>() on a non-const registry.

That said, I don't think this is your issue actually. You said that it runs fine a thousand times, then it breaks, right?
It's hard to tell you the root cause without a repro. Is your project public? Do you have a more detailed stack trace?
At first glance, it really seems as if the storage was invalidated and you were accessing a dangling reference but 🤷‍♂️ cannot confirm it without debugging it unfortunately, you know.

[skypjack]

Can I assume it was fixed? I don't have a repro nor other info since two weeks ago and I don't know how to help you further otherwise. So, I'm tempted to close the issue.

[JohnMrziglod]

Yes, thank you. It can be closed. I am sorry for not responding with proper info. I redesigned my code and probably eliminated my error, even though I don't know where it was exactly. Thank you very much for your suggestions.

[skypjack]

I'm glad you fixed it. Thanks for the quick turnaround. 👍