sourcemod icon indicating copy to clipboard operation
sourcemod copied to clipboard

FreeEvent call in EventManager::OnFireEvent breaks event pointer

Open Alexeyt89 opened this issue 3 years ago • 6 comments

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: Linux
  • Game/AppID (with version if applicable): 730
  • Current SourceMod version: 1.11.0.6842
  • Current Metamod: Source snapshot: 1.12.0-dev+1157
  • [X] I have updated SourceMod to the latest version and it still happens.
  • [X] I have updated SourceMod to the latest snapshot and it still happens.
  • [X] I have updated SourceMM to the latest snapshot and it still happens.

Description

If an event in a plugin is prehooked and blocked with Plugin_Handled the event is deleted in EventManager::OnFireEvent -> FreeEvent. So all the other hooks of IGameEventManager2::FireEvent receive dead IGameEvent pointer.

gameevents->FreeEvent(info.pEvent) should be removed from EventManager::OnFireEvent and EventManager::OnFireEvent_Post to fix this problem.

Alexeyt89 avatar Jan 03 '22 14:01 Alexeyt89

https://bugs.alliedmods.net/show_bug.cgi?id=5097 has the history here and a lot of discussion around the various options, unfortunately leaking memory is not a great option.

asherkin avatar Jan 03 '22 15:01 asherkin

If I understand right and the idea here is to prevent event transmitting when Plugin_Handled is returned then instead of deleting the event it can be made "local". There is a check in CGameEventTransmitter::TransmitGameEvent which prevents transmitting of local events. The IsLocal() function can be hooked and the return value can be changed to true. As events always get deleted at the end of CGameEventManager::FireEventIntern, it should be safe. Another way would be to add an interface for extensions so that developers wouldn't need to hook the FireEvent function.

Alexeyt89 avatar Jan 03 '22 20:01 Alexeyt89

Just blocking transmission of the event wouldn't cover all use cases, since listeners in the server itself would still receive it.

psychonic avatar Jan 03 '22 21:01 psychonic

Can we reference count the in-flight object?

dvander avatar Jan 03 '22 22:01 dvander

Or, wait until all notifications are fired and then free.

dvander avatar Jan 03 '22 22:01 dvander

Just blocking transmission of the event wouldn't cover all use cases, since listeners in the server itself would still receive it.

What if hook and block all IGameEventListener::FireGameEvent calls for such events?

Alexeyt89 avatar Jan 03 '22 22:01 Alexeyt89