FPtje / DarkRP

DarkRP, a non-serious roleplay gamemode for Garry's Mod.
https://darkrp.miraheze.org
MIT License
462 stars 707 forks source link

Error still happening #2665

Closed mcNuggets1 closed 7 years ago

mcNuggets1 commented 7 years ago

Description of the bug

[ERROR] gamemodes/darkrp/entities/entities/fadmin_jail/init.lua:26: Tried to use a NULL entity!

  1. SetPos - [C]:-1
    1. unknown - gamemodes/darkrp/entities/entities/fadmin_jail/init.lua:26

How to make the bug happen

Lua errors

Why the developer of DarkRP is responsible for this issue

Kefta commented 7 years ago

It looks like Replace is creating a NULL entity, which was the whole issue me and Falco discussed a few issues ago and he added a debugging-fix for. Are you sure that there is no error before that saying you hit the entity limit?

mcNuggets1 commented 7 years ago

3905 Entities

FPtje commented 7 years ago

#ents.GetAll() isn't a valid way to count entities. There might be fewer entities reported by #ents.GetAll() while still being on the limit.

There's no better way, so this'll have to do.

mcNuggets1 commented 7 years ago

I actually ran it serverside. How is that not trustworthy?

mcNuggets1 commented 7 years ago

There is not even a "hit entity" limit message.

FPtje commented 7 years ago

The function doesn't return all entities. Counting the table will therefore not count all entities.

mcNuggets1 commented 7 years ago

Does the new function ents.GetCount function better?

FPtje commented 7 years ago

Wiki says no.

Same as #ents.GetAll only with much better performance.

Bo98 commented 7 years ago

There is not even a "hit entity" limit message.

So no Not creating entity 'fadmin_jail' - too many edicts! (XXXX current, 8192 max) line in console?

mcNuggets1 commented 7 years ago

I just found out the error appeared after the server was shut down.

That has to be a gmod bug then.

"L 04/11/2017 - 20:26:41: Server successfully shut down L 04/11/2017 - 20:26:43: Lua Error: [ERROR] gamemodes/darkrp/entities/entities/fadmin_jail/init.lua:26: Tried to use a NULL entity!

  1. SetPos - [C]:-1
    1. unknown - gamemodes/darkrp/entities/entities/fadmin_jail/init.lua:26 "
mcNuggets1 commented 7 years ago

Well it crashed the server // 2018 - It actually locks the server, not crashing it.

FPtje commented 7 years ago

The error itself probably didn't crash the server. It would be more likely that something else caused both the crash and the error.

Bo98 commented 7 years ago

So it was gracefully shutting down, then the error occurred and then something happened that caused the server to crash?

Are you able to reproduce the error part every time by FAdmin jailing someone and then gracefully shutting down the server?

mcNuggets1 commented 7 years ago

It should have changed map, but due to the error shuts down as far as I can see.

Bo98 commented 7 years ago

OnShutdown is called on changelevel too so that would count as 'graceful shutdown' from a Lua point of view.

Still, can you reproduce the error by jailing someone and then changing the level while they are still jailed?

mcNuggets1 commented 7 years ago

It does not happen regularly.

But it nearly always errors on weapon_fists or the darkrp-jails.

Bo98 commented 7 years ago

What's the weapon_fists error, out of interest?

mcNuggets1 commented 7 years ago

I don't know it out of the head, because I fixed it, but it had to do with the UpdateIdle function.

Bo98 commented 7 years ago

I'm able to reproduce the jail error no problem.

Seems that GMod still calls OnRemove on entities when removing them on shutdown (or changelevel). This is causing an infinite loop in fadmin_jail, as it keeps trying to replace the removed entity. When GMod is in this state, it doesn't seem to block the creation of new entities, so it'll be adding entities to the entities list while GMod is removing them. The loop is only terminated by the entity limit.

DarkRP's error does not throw as #ents.GetAll() returns a constant value during this time, since ents.GetAll() does not include entities marked for removal, but the actual removal of the edict (and therefore the decrement in the number of entities counting towards the limit) only happens later. ents.GetCount() does not suffer this issue (though you'll have to wait a week for the update to release that includes it).

fadmin_motd has similar code but it doesn't actually work, since the target field is never set.

The reason you did not notice GMod's entity limit error message is because it never gets logged to the server logs. It appears in console only (and console.log probably if you have that enabled).

The crash is reproducible on the dev branch. It's caused by MySQLOO (though could be a bug with GMod's binary module changes) and it completely unrelated to the Lua error. In fact, it happens every time I changelevel when the server is not hibernating. I will report this.

FPtje commented 7 years ago

Oh wow

Bo98 commented 7 years ago

The behaviour of ends.GetCount() is getting changed unfortunately so switching that won't help the accuracy of the error in the long term:

https://github.com/Facepunch/garrysmod-requests/issues/953

Kefta commented 7 years ago

I posted a better solution, but FL_KILLME entities are only around for a tick, anyway, so it would only affects very rare edge cases. I would recommend printing a message before the user hits the entity limit to warn that they're getting close.

Bo98 commented 7 years ago

How long are edicts around for after FL_KILLME is set then? An arbitrary amount of time?

Bo98 commented 7 years ago

Or are you referring to the function not differentiating between networked and non-networked entities?

Kefta commented 7 years ago

Both, and the edict is around for a tick

Kefta commented 7 years ago
STORE = {}
local len = 1
local p = ents.Create("prop_physics")

repeat
    p:SetModel("models/player.mdl")
    p:Spawn()
    p:SetMoveType(MOVETYPE_NONE)
    p:SetCollisionGroup(COLLISION_GROUP_NONE)
    STORE[len] = p
    len = len + 1

    p = ents.Create("prop_physics")
until(p == NULL or len == 10000)

print(ents.GetCount(), ents.GetEdictCount(), #ents.GetAll())

Output:

Not creating entity 'prop_physics' - too many edicts! (8176 current, 8192 max)
8291    8176    8291

Seems like the new function is the way to go.

FPtje commented 7 years ago

Nice! I didn't know that function was added. Thanks @Kefta!