Skip to content

Fix concurrent writes to stdout in tests. #2350

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

trulede
Copy link
Contributor

@trulede trulede commented Aug 3, 2025

Writes to stdout from deps are occasionally "mixed up". This is caused by concurrent writes to the executor stdout variable by go routines (from the deps which run in parallel). The solution proposed here captures stdout to a buffer, and then transfers that buffer to the executor stdout with the protection of mutex to avoid concurrent writes.

The problem may be difficult to reproduce. My testing showed a 1-in-10 chance of the problem occurring when running the task tests. After correcting the issue I did not encounter failure for 30+ test runs.

fixes #2349
fixes #1208

@trulede trulede changed the title Fix concurant writes to stdout from deps. Fix concurrent writes to stdout from deps. Aug 4, 2025
Copy link
Member

@pd93 pd93 left a comment

Choose a reason for hiding this comment

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

Nice! This has been bothering me for a while. Thanks for investigating.

Copy link

netlify bot commented Aug 9, 2025

Deploy Preview for next-taskfile canceled.

Name Link
🔨 Latest commit 985e1ba
🔍 Latest deploy log https://app.netlify.com/projects/next-taskfile/deploys/68988a7fa0f6c000080715bb

@trulede trulede requested a review from andreynering August 9, 2025 21:35
@trulede
Copy link
Contributor Author

trulede commented Aug 10, 2025

I will have to tag the tests with "signals" (or delete them), its too difficult to get them running reliably in the CI.

Edit: New tests removed. Useful for developing this PR, but otherwise not fitting well.

@trulede trulede changed the title Fix concurrent writes to stdout from deps. Fix concurrent writes to stdout in tests. Aug 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Occasional flaky tests with grouped output. Grouped output is mixed with Task output
3 participants