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 race condition in ThreadedTaskScheduler when adding tasks #6014

Merged

Conversation

smoogipoo
Copy link
Contributor

@smoogipoo smoogipoo commented Oct 4, 2023

@smoogipoo smoogipoo changed the title Fix race condition in ThreadedTaskCondition when adding tasks Fix race condition in ThreadedTaskScheduler when adding tasks Oct 4, 2023
@smoogipoo smoogipoo force-pushed the fix-threadedtaskscheduler-race-condition branch from 841d4c9 to bd42383 Compare October 4, 2023 02:53
@peppy
Copy link
Sponsor Member

peppy commented Oct 4, 2023

Is there a reason the test coverage for this can't be committed?

@smoogipoo
Copy link
Contributor Author

How? It's a race condition... It's just fails at some random point.

@bdach
Copy link
Collaborator

bdach commented Oct 4, 2023

We've added such tests to source in the past. Compare:

public class ConvexPolygonClipperFuzzingTest

public partial class TestSceneStressGLDisposal : FrameworkTestScene

@peppy
Copy link
Sponsor Member

peppy commented Oct 4, 2023

Even if it's just run a few hundred times (and only fails one out of several runs), I think having the test around as a reference is valuable for any future manual testing.

@smoogipoo
Copy link
Contributor Author

TIL. Added the test.

@peppy peppy self-requested a review October 4, 2023 05:35
@pull-request-size pull-request-size bot added size/M and removed size/S labels Oct 4, 2023
@peppy peppy merged commit 907e62c into ppy:master Oct 4, 2023
17 of 21 checks passed
@smoogipoo smoogipoo deleted the fix-threadedtaskscheduler-race-condition branch October 10, 2023 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants