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

fix(game/five): updateBusySpinner thread constraint check #2952

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

Conversation

tens0rfl0w
Copy link
Contributor

@tens0rfl0w tens0rfl0w commented Nov 20, 2024

Goal of this PR

Prevent crashes caused by reading from a nullptr when updateBusySpinner is called from the Render thread.

How is this PR achieving the goal

Although the root cause of this issue requires further investigation, this PR provides a temporary fix to prevent crashes when updateBusySpinner is unexpectedly called from the Render thread instead of the Main thread. This call causes a subsequent function to return a nullptr due to failing thread constraint checks in native game code.

The only potential side effect of this fix is missing updates to the busy spinner during the loading of addon data files, but this is still better than outright crashing.

This PR applies to the following area(s)

FiveM

Successfully tested on

Game builds: 3258

Platforms: Windows (Client)

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.

Fixes issues

fixes #2850

This should work as a temporary fix to prevent crashes when for unknown reasons 'updateBusySpinner' gets called from the Render thread instead of the Main thread, which causes an inner function call to return a nullptr. due to failing thread constraint checks.
@github-actions github-actions bot added the triage Needs a preliminary assessment to determine the urgency and required action label Nov 21, 2024
@Mathu-lmn
Copy link
Contributor

Any plans to get this reviewed ? We're having new players reporting this issue

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.

updateBusySpinner leads to crash
2 participants