pcm_file: retry write(2) on short writes in safe_write() - #521
Open
Krishnanand-G wants to merge 1 commit into
Open
pcm_file: retry write(2) on short writes in safe_write()#521Krishnanand-G wants to merge 1 commit into
Krishnanand-G wants to merge 1 commit into
Conversation
write(2) on a pipe or FIFO can return fewer bytes than requested. This happens when a blocking write gets interrupted by a signal after part of the buffer already went through: the kernel hands back the partial count instead of -EINTR. safe_write() treated any non-negative return as complete and passed that short count straight back to the caller. snd_pcm_file_write_bytes() then saw err != n, broke out of its write loop, and returned success anyway, so the unwritten tail of the period never reached the target file. Writes to a FIFO block waiting for a reader far more often than writes to a regular file do, which is exactly why the file plugin only drops samples on pipes and works fine on plain files. Make safe_write() loop until every byte is written or a real error turns up. Fixes: alsa-project#63 Signed-off-by: Krishnanand G <krishnanandgeetheswaran@gmail.com>
Krishnanand-G
force-pushed
the
fix-file-plugin-short-write
branch
from
August 21, 2026 09:56
4e6c6f8 to
6ab4aec
Compare
Author
|
The signed-off check passes. This is a small safety fix in safe_write(), retrying short writes so a partial write does not silently lose data. I would appreciate a maintainer review when possible. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #63.
The file plugin drops audio when its output is a FIFO but not when it's a regular file. Looking through pcm_file.c, safe_write() only retries write(2) when it returns -1/EINTR. On a pipe, a blocking write that's interrupted by a signal after part of the buffer already went through returns the partial byte count instead, not -1. safe_write() passed that short count straight back, and snd_pcm_file_write_bytes() treats "wrote less than requested" as a reason to stop and return success, so the rest of that period's samples never made it out.
That also matches why regular files never showed the bug: a write to a file rarely blocks long enough for a signal to land mid-syscall, while a write to a FIFO sits blocked waiting on a reader constantly.
The fix is small: make safe_write() keep writing until the whole buffer is out or a real error comes back, instead of stopping on the first partial write.
I wrote a small standalone repro outside the tree (pipe with a small buffer + slow reader + SIGALRM firing during the write) to confirm the mechanism before touching pcm_file.c. With the old loop the writer reliably loses most of a 512KB buffer (~12KB gets through per run); with the retry loop all 512KB gets through every time despite the same interruptions. Also ran a full
./configure && makeof the library to confirm it still builds clean.I didn't have a real FIFO consumer / hardware set up to exercise this through actual aplay/arecord, so if anyone hitting #63 wants to try this against their original setup, that would help confirm it end to end.