No exception when releasing handles owned by other plugins

[KillStr3aK]

Help us help you

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

Environment

• Operating System version: Windows Server 2019
• Game/AppID (with version if applicable): CS:GO
• Current SourceMod version: 1.12.0.6972
• Current Metamod: Source snapshot: 1.12.0-dev+1165

• [ ] I have updated SourceMod to the latest version and it still happens.
• [ ] I have updated SourceMod to the latest snapshot and it still happens.
• [ ] I have updated SourceMM to the latest snapshot and it still happens.

Description

During investigation, me and @nickdnk encountered an issue. We're not completely sure if this is sourcemod related but we don't lose nothing with opening this issue. Note that this is only happening when 2 plugin shares the same handle. Using the same method in a single plugin works as intended.

This issue relies on the sm-json library but might be pretty unrelated to it in general. This library has nested handles, and when PLUGIN1 shares a handle with PLUGIN2, and PLUGIN2 creates a new handle inside the first one that was shared, and is owned by PLUGIN1 (but the new handle is owned by PLUGIN2), then when you want to release the handle (that was created and owned by PLUGIN2) in PLUGIN1 sourcemod shouldn't allow it (and it works as intended) but there should be an exception that you cannot release handles owned by other plugins (atleast I guess?) but there is not, and without further information it leaks memory. This description might be confusing so lets go to the code part where the comments might help for better understanding.

Problematic Code (or Steps to Reproduce)

// PLUGIN A:
#include <json>

GlobalForward gForward;

public void OnPluginStart()
{
  gForward = new GlobalForward("NYXI_JSON", ET_Ignore, Param_Cell);

  // new JSON_Object handle is created here
  JSON_Object obj = new JSON_Object();

  Call_StartForward(gForward);

  // passed to the forward (PLUGIN B, ... PLUGIN C ...)
  Call_PushCell(obj);

  // Before further reading, read the comments in PLUGIN B
  Call_Finish();

  // So when we get back there, 'obj' has a handle in 'obj.Snap' that is owned by PLUGIN X
  // Fully release 'obj' and all its nested handles BUT since 'obj.Snap' is not created
  // by this plugin, the given handle is not listed in the dump which is intented (but it is listed for the other plugin)
  // BUT when releasing it (there: https://github.com/clugg/sm-json/blob/master/addons/sourcemod/scripting/include/json/object.inc#L111)
  // it shouldn't be able to release it and that is where we should get an exception?
  // the handle is still the same in PLUGIN X and leaks memory in PLUGIN X
  json_cleanup_and_delete(obj);
}

// PLUGIN B:
#include <json>

// 'const' is not related to the issue.
public void NYXI_JSON(const JSON_Object obj)
{
  // 'obj.Iterate' creates a new handle inside the 'obj' variable called 'Snap' which is a StringMap::Snapshot in general
  // and this 'Snap' handle is lingering until the object is released in the 'json_cleanup_and_delete' function.
  //
  // But here comes the issue:
  // We're in PLUGIN B now and the 'obj' is from PLUGIN A.
  // 'obj.Iterate' is creating the new Handle there: https://github.com/clugg/sm-json/blob/master/addons/sourcemod/scripting/include/json/object.inc#L115
  // And normally, it is released there: https://github.com/clugg/sm-json/blob/master/addons/sourcemod/scripting/include/json/object.inc#L110-L112
  // REST IS HAPPENNIG IN PLUGIN A
  obj.Iterate();
}

Logs

Handle dump after reloading PLUGIN A in X amount of times (which starts the forward on 'OnPluginStart') pluginB.smx - TrieSnapshot | 7 handles | 1036 bytes (THIS LEAKS) pluginA.smx - GlobalFwd | 1 handles | 16 bytes (GOOD)

[asherkin]

This appears to be an issue with the library you're using not being explicit about handle ownership in the natives that you're using.

Delete intentionally suppresses an error from invalid ownership, I can't remember why.

[KillStr3aK]

This appears to be an issue with the library you're using not being explicit about handle ownership in the natives that you're using.

Yes.

Delete intentionally suppresses an error from invalid ownership, I can't remember why.

This is what I'm missing currently. Even a warning could do it that says you tried to release something you have no access to, so it won't break existing plugins that may have this issue (if there is any)

[KyleSanderson]

This appears to be an issue with the library you're using not being explicit about handle ownership in the natives that you're using.

Delete intentionally suppresses an error from invalid ownership, I can't remember why.

I think it's from Events and similar

[nickdnk]

This appears to be an issue with the library you're using not being explicit about handle ownership in the natives that you're using. Delete intentionally suppresses an error from invalid ownership, I can't remember why.

I think it's from Events and similar

This has indeed been resolved in v5 of sm-json: https://github.com/clugg/sm-json/releases/tag/v5.0.0, but a warning from SourceMod when attempting to delete an unowned handle would definitely have helped figuring this out.