René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Crash due to ConVar overlap and release. #421","articleBody":"This is an issue derived from #421 and introduced by 02e34907fc86036228d132526edcd8f847a0650a. There is a long discussion going on in #421, and I've created an issue for the record.\r\n\r\nThe main issue is due to Source.Python releasing the ConVar object created by [ConVar(str)](https://github.com/Source-Python-Dev-Team/Source.Python/blob/02e34907fc86036228d132526edcd8f847a0650a/src/core/modules/cvars/cvars_wrap.cpp#L207) when the plugin is unloaded. This will cause other Source.Python/SourceMod plugins to crash if they hold the same ConVar object.\r\n\r\nCode:\r\n\r\nPlugin1 (test1)\r\n```python\r\nfrom cvars import ConVar\r\ntest_convar = ConVar(\"test_convar\", \"1\", \"Test Convar(test1).\")\r\n```\r\n\r\nPlugin2 (test2) Or a SourceMod plugin with equivalent functionality.\r\n```python\r\nfrom commands.server import ServerCommand\r\nfrom cvars import ConVar\r\ntest_convar = ConVar(\"test_convar\", \"2\", \"Test Convar(test2).\")\r\n\r\n@ServerCommand(\"set_test\")\r\ndef test_command(*args):\r\n    test_convar.set_string(\"3\")\r\n```\r\n\r\nOutput;\r\n```\r\n$ sp plugin load test1\r\n[SP] Loading plugin 'test1'...\r\n[SP] Successfully loaded plugin 'test1'.\r\n\r\n$ test_convar\r\n\"test_convar\" = \"1\" - Test Convar(test1).\r\n\r\n$ sp plugin load test2\r\n[SP] Loading plugin 'test2'...\r\n[SP] Successfully loaded plugin 'test2'.\r\n\r\n$ sp plugin unload test1\r\n[SP] Unloading plugin 'test1'...\r\n[SP] Successfully unloaded plugin 'test1'.\r\n\r\n$ set_test\r\nThread 1 \"srcds_linux\" received signal SIGSEGV, Segmentation fault.\r\n0xf076d5e5 in boost::python::objects::polymorphic_id_generator\u003cConVar\u003e::execute(void*) ()\r\n```\r\n\r\nThe same problem occurs with the wrapping plugin for ConVars.\r\n\r\nThe root of the problem lies in the fact that ConVar(str) returns an existing registered ConVar.\r\nhttps://github.com/Source-Python-Dev-Team/Source.Python/blob/02e34907fc86036228d132526edcd8f847a0650a/src/core/modules/cvars/cvars.h#L95-L104\r\n\r\nIn 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-\u003eFindVar) 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.\r\n\r\nUnfortunately, 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.\r\n\r\nBut 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 02e34907fc86036228d132526edcd8f847a0650a, but should not be disabled for warnings.)\r\nhttps://github.com/Source-Python-Dev-Team/Source.Python/blob/02e34907fc86036228d132526edcd8f847a0650a/src/core/modules/commands/commands_server.cpp#L54-L62\r\n\r\nWe 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.","author":{"url":"https://github.com/CookStar","@type":"Person","name":"CookStar"},"datePublished":"2021-10-30T09:58:08.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":2},"url":"https://github.com/430/Source.Python/issues/430"}