Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tweak(conhost): use circular_buffer instead of ImVector #2728

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AvarianKnight
Copy link
Contributor

@AvarianKnight AvarianKnight commented Aug 19, 2024

Goal of this PR

Fix crash when printing a large amount of things to console (2501+) due to ItemTimes using a ImVector instead of circular_buffer.

-- this expects you to have run `con_miniconChannels "script:*"` prior
CreateThread(function()
  for i = 1, 5000 do
    print("test", i)
  end
end)

How is this PR achieving the goal

Replace ImVector with a circular_buffer so that it properly caps out at 2500 items instead of allowing it to infinity expand as a Vector, causing a crash later when it tries to remove Items and ItemKeys because they are properly restricted to 2500 items and will try to remove past the bounds if Items.

while ((ItemTimes.size() > 1 && (t - ItemTimes[0]) > std::chrono::seconds(10)) || Items.size() > 14)
{
Items.erase(Items.begin());
ItemKeys.erase(ItemKeys.begin());
ItemTimes.erase(ItemTimes.begin());
}

This PR applies to the following area(s)

FiveM/RedM

Successfully tested on

Game builds: N/A

Platforms: N/A

Checklist

  • Code compiles and has been tested successfully.
  • Code explains itself well and/or is documented.
  • My commit message explains what the changes do and what they are for.
  • No extra compilation warnings are added by these changes.

- `Items` and `ItemKeys` use a circular buffer and can only store `2500` items, but ItemTimes could expand and store more.
- This would cause a crash at https://github.com/citizenfx/fivem/blob/705cd7901ac03c5a0d1e1355d94e84046309b80d/code/components/conhost-v2/src/ConsoleHostGui.cpp#L729-L734 because `ItemTimes` would have a capacity of 2500+ but `Items` and ItemKeys` would be empty
@github-actions github-actions bot added the triage Needs a preliminary assessment to determine the urgency and required action label Aug 19, 2024
@FabianTerhorst FabianTerhorst self-assigned this Aug 19, 2024
@FabianTerhorst FabianTerhorst self-requested a review August 19, 2024 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Needs a preliminary assessment to determine the urgency and required action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants