Skip to content

Comments

Make OnEndOfFrame thread safe#928

Merged
maddie480 merged 1 commit intoEverestAPI:devfrom
wuke32767:safe-end-of-frame
Jun 30, 2025
Merged

Make OnEndOfFrame thread safe#928
maddie480 merged 1 commit intoEverestAPI:devfrom
wuke32767:safe-end-of-frame

Conversation

@wuke32767
Copy link
Contributor

Previously: Any OnEndOfFrame added during OnEndOfFrame invocation will be discarded.

Changes: They will be invoked in next frame.

Note: OnEndOfFrame can add new OnEndOfFrames. Nobody should be doing this (otherwise they will notice it), but if it happens, this pr causes desync.

@maddie480-bot maddie480-bot added the 1: review needed This PR needs 2 approvals to be merged (bot-managed) label Jun 20, 2025
Copy link
Member

@JaThePlayer JaThePlayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't really say it's about thread-safety as its more about being able to set OnEndOfFrame on the same thread, but I'm for this either way. (With that said, Interlocked here isn't really necessary, but the code's cleaner than the non-Interlocked way, so it's fine imo)

@wuke32767
Copy link
Contributor Author

It's initially for thread safety, but the listed changes are more significant.

@maddie480-bot
Copy link
Member

The pull request was approved and entered the 3-day last-call window.
If no further reviews happen, it will end on Jun 29, 2025, 12:00 AM UTC, after which the pull request will be able to be merged.

@maddie480-bot maddie480-bot added 3: last call window This PR was approved, and is in the 5-day last-call window before getting merged (bot-managed) and removed 1: review needed This PR needs 2 approvals to be merged (bot-managed) labels Jun 23, 2025
@maddie480-bot
Copy link
Member

The last-call window for this pull request ended. It can now be merged if no blockers were brought up.

@maddie480-bot maddie480-bot added 4: ready to merge This PR was approved and the last-call window is over (bot-managed) and removed 3: last call window This PR was approved, and is in the 5-day last-call window before getting merged (bot-managed) labels Jun 28, 2025
@maddie480 maddie480 merged commit bd8fedb into EverestAPI:dev Jun 30, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4: ready to merge This PR was approved and the last-call window is over (bot-managed)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants