Skip to content

Crash due to ConVar overlap and release. #421 #430

Closed
@CookStar

Description

@CookStar

This is an issue derived from #421 and introduced by 02e3490. There is a long discussion going on in #421, and I've created an issue for the record.

The main issue is due to Source.Python releasing the ConVar object created by ConVar(str) when the plugin is unloaded. This will cause other Source.Python/SourceMod plugins to crash if they hold the same ConVar object.

Code:

Plugin1 (test1)

from cvars import ConVar
test_convar = ConVar("test_convar", "1", "Test Convar(test1).")

Plugin2 (test2) Or a SourceMod plugin with equivalent functionality.

from commands.server import ServerCommand
from cvars import ConVar
test_convar = ConVar("test_convar", "2", "Test Convar(test2).")

@ServerCommand("set_test")
def test_command(*args):
    test_convar.set_string("3")

Output;

$ sp plugin load test1
[SP] Loading plugin 'test1'...
[SP] Successfully loaded plugin 'test1'.

$ test_convar
"test_convar" = "1" - Test Convar(test1).

$ sp plugin load test2
[SP] Loading plugin 'test2'...
[SP] Successfully loaded plugin 'test2'.

$ sp plugin unload test1
[SP] Unloading plugin 'test1'...
[SP] Successfully unloaded plugin 'test1'.

$ set_test
Thread 1 "srcds_linux" received signal SIGSEGV, Segmentation fault.
0xf076d5e5 in boost::python::objects::polymorphic_id_generator<ConVar>::execute(void*) ()

The same problem occurs with the wrapping plugin for ConVars.

The root of the problem lies in the fact that ConVar(str) returns an existing registered ConVar.

ConVar *pConVar = ICVarSharedExt::FindVar(g_pCVar, name);
if (!pConVar)
{
ConVar* pConVar = new ConVar(strdup(name), strdup(value), flags,
strdup(description), !min_value.is_none(), fMin, !max_value.is_none(), fMax);
return boost::shared_ptr<ConVar>(pConVar, &Deleter);
}
return boost::shared_ptr<ConVar>(pConVar, &NeverDeleteDeleter<ConVar *>);

In the Valve Server Plugin, the ConVar used by the plugin is created each time it is loaded, and released when it is unloaded. Also, when accessing a ConVar that already exists, the plugin retrieves the ConVar from ICvar.FindVar(g_pCVar->FindVar) and checks if it is valid before accessing it. However, SourcePython's ConVar(str) and SourceMod's CreateConVar return an existing ConVar, so if there are overlapping ConVars, intentionally or unintentionally, the plugin will own a single ConVar, resulting in a crash.

Unfortunately, ConVar(str) is used by many plugins to access an existing ConVar instead of cvar.find_var which should be used, and it is not possible to not return an existing ConVar without causing a major compatibility break.

But either way, when ConVar overlaps, the parent is set and the children access the parent, so when the parent is released, this also causes a crash. (This has been disabled in 02e3490, but should not be disabled for warnings.)

virtual bool RegisterConCommandBase(ConCommandBase* pCommand)
{
if (!g_pCVar->FindCommandBase(pCommand->GetName())) {
g_pCVar->RegisterConCommand(pCommand);
return true;
}
return false;
}

We should stop releasing ConVar when unloading plugins, and if we do release it, at least it should be done when unloading Source.Python VSP, so that plugins can work safely. However, if we want to be compatible with SourceMod, I think we should prohibit release of ConVar itself.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions