Problem with remove_touch_links

[cravenge]

As stated in the issue title, the plugin seems to crash the server whenever a bot with an idle human player dies. This happens mostly in the last two maps of No Mercy where getting karma charged is certain to occur. Here is the crash log:

0 server.dll + 0x1fcb28 1 server.dll + 0x1feef2 2 server.dll + 0x14c42c 3 server.dll + 0x14c48a 4 server.dll + 0x141e27 5 server.dll + 0x144254 6 server.dll + 0xeddeb 7 sourcemod.2.l4d2.dll + 0x15533 8 engine.dll + 0x124577 9 engine.dll + 0x1256b7 10 engine.dll + 0x18f398 11 engine.dll + 0x190c06 12 engine.dll + 0x191363 13 engine.dll + 0x1a0a2b 14 engine.dll + 0x1a0b7f 15 engine.dll + 0x1a0c24 16 engine.dll + 0x200cf4 17 engine.dll + 0x1fe311 18 dedicated.dll + 0x31ce 19 dedicated.dll + 0x3906 20 dedicated.dll + 0x27dae 21 dedicated.dll + 0x4976 22 srcds.exe + 0x11fd 23 srcds.exe + 0x1929 24 kernel32.dll!BaseThreadInitThunk + 0x19 25 ntdll.dll!__RtlUserThreadStart + 0x2f 26 ntdll.dll!_RtlUserThreadStart + 0x1b

[shqke]

I've tried to kill off a bot (that I was idling as) by a charger from a c8m5 rooftop - no crashes on linux or windows. Call trace frame offsets don't match with instruction on latest binaries for windows.

Could you update a call trace on a latest game version, or at least mention which game version you were using?

[cravenge]

Sorry, I was gonna report this before but forgot about it, thinking it was the faulty SourceTV Support extension causing it randomly at the time. The server was running on the version before June 15 (2.2.1.3) so I think either 2.2.1.1 or 2.2.1.2? I can't test for now ever since my server has been offline after that.

I can still remember how the crash happened:

1. Have a bot with an idle human player stand on any of the edges.
2. Let a Charger charge them off the building and into the ground where it's full of trigger_hurt entities.
3. Server crashes as soon as the bot dies.

[shqke]

It's alright, version number 2.2.1.2 helped.

I've verified a spot - it was an anticipated issue.

My plugin is calling CBaseEntity::PhysicsRemoveTouchedList on player_team event which recursively unlinks nodes from a linked "touch" list, which can call CTriggerHurt::EndTouch (if player was touching a trigger_hurt and entity damage filter passes) which does damage to untouching player in relation with trigger_hurt.

The other day we have discussed with @a1mdev an issue related to modding with this method and a possible workaround.

In your case linked "touch" list loses its integrity, this can happen if theres a team change is triggered on touching player, right after CTriggerHurt::EndTouch, - which invokes CBaseEntity::PhysicsRemoveTouchedList and removes links that are being iterated over. Crash happens in CTriggerHurt think function when CTriggerHurt::HurtAllTouchers is trying to iterate over a broken linked list.

A team change could also be caused by an immediate kick (KickClientEx). To confirm, I'd need to know your plugin list (sm plugins list).

A solution would be to add a delay before forcing a team change, something like:

...
void Handler_DelayedChangeTeam(int data)
{
    int client = GetClientOfUserId(data & 0xFFFF);
    if (client == 0) {
        return;
    }

    if (!IsClientConnected(client)) {
        return;
    }

    ChangeClientTeam(client, data >> 16);
}
...
void somedamagehandler(int client, ...)
{
...
    int newTeamIndex = ...;
...
    RequestFrame(Handler_DelayedChangeTeam, GetClientUserId(client) | ( newTeamIndex << 16 ));
...
}
...

For majority of cases it should be safe.

[cravenge]

I don't have any plugins that utilizes the KickClientEx function or anything else that changes a player's team immediately. The game automatically makes the idle human player takeover their bot when they die iirc. I think that's the forced team change you're talking about.

[shqke]

Yes, that could explain the crash, but I cannot reproduce it, which could indicate a mod involvement.

The game automatically makes the idle human player takeover their bot when they die iirc. I think that's the forced team change you're talking about.

In my case (coop, c8m5) a team change happens after player_death, so my trigger doesn't "hurt" on EndTouch, but does on "Touch". Meaning that my character doesn't get to trigger CBaseEntity::PhysicsRemoveTouchedList within CBaseEntity::PhysicsRemoveTouchedList on some related "EndTouch", yet it somehow happens on your side?

Are you able to reproduce a bug with the only remove_touch_links plugin loaded, without anything interfering?

For your convenience, heres the code of a sample plugin that adds a fake player, letting you go idle.

#include <sourcemod>

public Action sm_fake(int client, int args)
{
    int bot = CreateFakeClient("fake client");
    ChangeClientTeam(bot, 2);

    ServerCommand("mp_disable_autokick %d", GetClientUserId(bot));
}

public void OnPluginStart()
{
    RegAdminCmd("sm_fake", sm_fake, ADMFLAG_ROOT);
}

[lunatixxx]

So basically if any plugin kick or ban a player at the right moment it could crash the server? Of if the player change team at the right moment ?

[shqke]

@cravenge I apologize for the delay.

I was able to reproduce your issue. I have no idea why it didn't work the other time. 😃

So basically if any plugin kick or ban a player at the right moment it could crash the server? Of if the player change team at the right moment ?

There's a reason for KickClient being delayed, - less safe counterpart KickClientEx may cause a crash if used at wrong moment (even same broken list could be the cause).

CTerrorPlayer::ChangeTeam should have a frame delay to keep nested calls "grounded" - in case exact or similar in severity condition happens, just to be safe.

[shqke]

b3da301 should fix the issue.