cached entity table for ents.Iterator() can contain null entities (clientside-only)

[mgetJane]

Details

extremely rarely, ents.Iterator() will iterate over some invalid entities clientside

this is fixed when the entity cache gets invalidated again, but it should never contain null entities

this happened when i triggered a map cleanup

Steps to reproduce

not sure where to even begin since it appears to be caused by some sort of race condition involving map cleanups or full updates

this happened literally only once so far in the many hundreds of times that i've triggered map cleanups since the march update, and there was nothing special whatsoever with that particular map cleanup

[mgetJane]

before anyone says it, nope, using IsValid checks is NOT a solution for this

if the cached table has invalid entities, then it means that the cache is outdated, which defeats the point of ents.Iterator() being a replacement for ipairs(ents.GetAll())

[mgetJane]

invalidating the cache in Pre/PostCleanupMap and/or OnRequestFullUpdate can be potential solutions, but they kinda feel like shots in the dark without knowing exactly what the nature of the issue is

my guess is that how the EntityRemoved hook internally works might be where the issue originates

since idk how to reproduce it, i guess here's more info on when it happened: the gamemode was TTT with a relatively very minimal amount of addons and the map cleanup wasn't caused by a map restart, i just manually did it with game.CleanUpMap()

btw not sure if it's relevant, but the server also had sv_parallel_sendsnapshot 1

[mgetJane]

forgot to mention, there were no lua errors caused in the OnEntityCreated and EntityRemoved hooks when this happened (if some addon errors in those hooks, then the cache is likely to get screwed up because hooks are currently called in an arbitrary order)

[robotboy655]

This might be related to this unhelpfully titled issue:
#5792

Any way to reproduce the issue would go a long way to fixing it. Errors during hooks is a valid point.

[mgetJane]

triggered it again on a map cleanup on a different map (and again, nothing special whatsoever)

i printed out what's inside select(2, ents.Iterator()) when this happened and it had 2 null entities in the table

i should've printed out what was also inside ents.GetAll() to compare them, but unfortunately i absent-mindedly crossed a visleaf which networked some more entities to my client so the cached table was cleared before i could do that

probably not useful, but this is the before and after: https://diffchecker.com/5dJ5Vq2i/

all the entities above the null entities on the list are entities that persist between cleanups (players, weapons that players are carrying, etc) and everything else below are the map entities that respawn for every cleanup

[mcNuggets1]

Anything happening here or should we just not use the iterators?

[robotboy655]

I have yet to see a way to reproduce the issue.

[mgetJane]

Anything happening here or should we just not use the iterators?

it should be safe to use ents.Iterator clientside if you use IsValid (player.Iterator is fine)

i got no idea yet if the table that's being iterated over is up to date though when this bug occurs

[FPtje]

Some PRs to actually use these functions in gmod and DarkRP are blocked on this issue. What is the next step? Does this issue still occur, was it fixed in an update?

If this still happens, you could probably try to reproduce it by comparing the iterator to ents.GetAll after cleaning up the map, creating some clientside entities and whatnot.

[robotboy655]

I have not changed anything that would address this, as again, I do not know of a way to even remotely reproduce this, and people who reported the issue initially, who this has allegedly happened to, are now silent for almost 2 months.

If I had to guess, the issue is either this: #5792 (which is also silent for 2 months) or some race condition.

[FPtje]

Thanks for the update. I reckon a next step would be to write a canary script: on some timer, check the iterator for NULL entities. Once it finds them, it would look through a previously stored iteration and compare, to see if info from the missing entities can be extracted. Class names, whatnot.

That could give a clue as to which entities are affected. As for the iterator, I guess a good hard stare at the code could help?

[FPtje]

@mgetJane could you run a canary script?