Private forwards crash when freed mid-call on specific platforms

[nosoop]

Prerequisites

• I have checked that my issue doesn't exist yet.
• I have tried my absolute best to reduce the problem-space and have provided the absolute smallest test-case possible.
• I can always reproduce the issue with the provided description below.

Environment

• Operating System version: Debian 10 (Buster. 64-bit)
• SourceMod version: 1.9.0.6281

Description

Server crashes if the private forward handle being called is deleted during one of the callbacks in Call_Finish(). This is something that NativeVotes does to be similar in design to the native Menu handle type. Definitely fixable there, but there probably should be safeguards in place on the SourceMod side.

This occurs on my "production" server instance of Debian 10; unfortunately Steam's content servers aren't cooperating and so I can't create a clean Debian 10 environment to test in.

I have not seen this issue on other operating systems.

More information here.

Problematic Code (or Steps to Reproduce)

#pragma semicolon 1
#include <sourcemod>

#pragma newdecls required

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

public Action CallFreedForward(int client, int argc) {
  Handle fwd = CreateForward(ET_Ignore, Param_Cell);
  AddToForward(fwd, INVALID_HANDLE, ForwardCall);
  
  Call_StartForward(fwd);
  Call_PushCell(fwd);
  Call_Finish();
  
  PrintToServer("Finished call");
}

public void ForwardCall(Handle fwd) {
  // this is similar to what the nativevotes menu does -- it frees the handler on its callback
  delete fwd;
}

Call call_freed_forward through the server console. As previously noted, this plugin crashes on my instance of Debian 10, and does not crash on Ubuntu 16.04 64-bit.

The offending plugin did not crash on Debian 9, so I would assume this code also doesn't.

Logs

Crash occurs here. The call stack is otherwise inconsistent and dependent on what the offending plugin was doing.

[asherkin]

Nice job tracking this one down!

[KyleSanderson]

I'm up in the Arctic Circle so I won't have time to look at this for a bit. I suspect a similar solution here for you would to be to look at this.

#268

[nosoop]

Yeah, that looks pretty much like the plugin-side fix for it.

If my understanding of core/logic/smn_functions.cpp is correct, there's no way to do nested calls of Call_StartForward, right? In that case I can probably add in a static Handle_t s_ForwardHndl and clone / free it in the appropriate functions and test that out.

[KyleSanderson]

Yeah, it's a one-shot because this is a huge performance bottleneck. If you're reckless it's going to cause problems like this, but there's a huge (documented) hit already from taking this path. I have an entire ecosystem that turned out lighter and more dynamic because I was super careful with what each function was doing but it's absolutely more expensive then direct calling (it was something like 10x 7 years ago, the gap has likely narrowed since).