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 log printer #11286

Merged
merged 1 commit into from Dec 20, 2023

Conversation

horus
Copy link
Contributor

@horus horus commented Dec 19, 2023

What I did

The code used an atomic bool to guard channel writes. However, this failed to synchronize with the call to close(), causing a panic.

Fix the race condition by using a mutex to guard the update to the bool stopped and subsequent channel writes. This ensures atomic execution of both updates to stopped and channel writes, preventing races between writes and close().

Related issue

Closes #11280

(not mandatory) A picture of a cute animal, if possible in relation to what you did
IMG_1463

The code used an atomic bool to guard channel writes. However, this
failed to synchronize with the call to close(), causing a panic.

Fix the race condition by using a mutex to guard the update to the
bool `stopped` and subsequent channel writes. This ensures atomic
execution of both updates to `stopped` and channel writes, preventing
races between writes and close().

Signed-off-by: horus <horus.li@gmail.com>
@horus horus force-pushed the fix/logprinter-channel-race branch from 95b9cab to 20fe2f3 Compare December 19, 2023 15:51
@ndeloof
Copy link
Contributor

ndeloof commented Dec 20, 2023

I (for once!) got this exact same issue easily reproduced with a test compose.yaml file and confirmer this fix it, thanks

@ndeloof ndeloof enabled auto-merge (rebase) December 20, 2023 14:37
@ndeloof ndeloof changed the title up: fix write/close race condition in logPrinter Fix race condition in log printer Dec 20, 2023
@ndeloof ndeloof merged commit 1baa4f4 into docker:main Dec 20, 2023
24 checks passed
@horus
Copy link
Contributor Author

horus commented Dec 20, 2023

Also as a daily user, I'm glad that this issue could be resolved in just a few days.
@ndeloof, your quick responses and advice were very helpful. I really appreciate it.

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.

[BUG] up: logPrinter panicked with "send on closed channel"
2 participants