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: thread safety issue during exiting the game #9380

Merged
merged 1 commit into from Jun 17, 2021

Conversation

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Jun 17, 2021

Motivation / Problem

ThreadSantizer was complaining on exiting the game. It is a bit silly, as it is a boolean value .. but making it a std::atomic is also a small effort.

Description

_exit_game is read by the draw-thread to know when to exit, but
most of the time written by the game-thread.

Most likely we could change all the loads and stores to memory_order_relaxed, as I doubt it really matters honestly. We are pretty resilient against _exit_game being flipped at the weirdest places.

The reason I didn't do it, is because there are roughly 30 places where we load/store _exit_game. That maybe highlights a completely different issue in our codebase :P

Limitations

  • I am never all to sure about the performance impact of making something into a std::atomic. I would think it doesn't matter (at all) in this case, but I would love to be told wrong here :)

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')
_exit_game is read by the draw-thread to know when to exit, but
most of the time written by the game-thread.
@TrueBrain TrueBrain merged commit c12a152 into OpenTTD:master Jun 17, 2021
14 checks passed
Loading
@TrueBrain TrueBrain deleted the random-thread-safe branch Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants