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

#11771 make cfreactor less flaky #12160

Merged
merged 12 commits into from
May 3, 2024
Merged

Conversation

glyph
Copy link
Member

@glyph glyph commented May 3, 2024

Scope and purpose

Fixes #11771

This is the change that appeared to have made a difference isolated and extracted from #12150 .

@glyph
Copy link
Member Author

glyph commented May 3, 2024

Both of the failures are in twisted.internet.test.test_tcp.AbortConnectionTests_CFReactorTests.test_fullWriteBuffer, which also fails sometimes in the asyncioreactor too: #12151

so I think this fix actually resolves the issue

@glyph
Copy link
Member Author

glyph commented May 3, 2024

please review

@chevah-robot chevah-robot requested a review from a team May 3, 2024 08:42
Copy link
Member

@adiroiban adiroiban left a comment

Choose a reason for hiding this comment

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

Thanks. Many, many thanks for working on this.

Were you able to reproduce the errors on your local system?

I was trying to reproduce them on a macmini m1, without luck.


I think that we should just merge it and see if works.

Copy link
Member

@adiroiban adiroiban left a comment

Choose a reason for hiding this comment

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

Previous comment was an "approve".

@adiroiban adiroiban merged commit 3eba981 into trunk May 3, 2024
24 checks passed
@adiroiban adiroiban deleted the 11771-cfreactor-remove-force-flag branch May 3, 2024 09:47
@glyph
Copy link
Member Author

glyph commented May 3, 2024

Were you able to reproduce the errors on your local system?

I was not. But in every other case I managed to reproduce the error in CI after a minimum of 5 tries.

The race condition here is extremely subtle, and I couldn't even think of a white-box test to reproduce it, but basically if something were to call _scheduleSimulate without force after calling it with force, so logically I think I see how this hang could happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants