Skip to content

input: wake callers on errors and EOF - #2576

Merged
MaxKellermann merged 3 commits into
MusicPlayerDaemon:v0.24.xfrom
acts-1631:fix/async-input-notification
Aug 24, 2026
Merged

input: wake callers on errors and EOF#2576
MaxKellermann merged 3 commits into
MusicPlayerDaemon:v0.24.xfrom
acts-1631:fix/async-input-notification

Conversation

@acts-1631

Copy link
Copy Markdown
Contributor

AsyncInputStream::Read() waits on caller_cond, but EOF and error callbacks in asynchronous input implementations only called InvokeOnAvailable(). After bd7830794, that no longer wakes synchronous readers. Commands that enumerate a truncated remote playlist can therefore leave MPD's main thread blocked indefinitely.

Make AsyncInputStream notify caller_cond whenever it invokes the availability handler, covering all asynchronous implementations. Remove noexcept from the playlist enumeration helpers so I/O errors reach the command handler instead of terminating MPD after the reader is woken.

CommitWriteBuffer(nbytes);
}
catch (...) {
const std::lock_guard protect{mutex};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change is correct, but hiding it inside this unrelated commit is not. This should be a separate commit with a commit message explaining the fix.

Comment thread src/playlist/Length.cxx
const SongLoader &loader,
const char *uri,
SongEnumerator &e) noexcept
SongEnumerator &e)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Huh? Unrelated undocumented change?

Comment thread src/playlist/Print.cxx
unsigned start_index,
unsigned end_index,
bool detail) noexcept
bool detail)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Huh? Unrelated undocumented change?

Comment thread src/playlist/Print.cxx
unsigned start_index,
unsigned end_index,
SongFilter *filter) noexcept
SongFilter *filter)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Huh? Unrelated undocumented change?

SongEnumerator::NextSong() can throw while reading a remote playlist.
The provider helpers are called by functions that already propagate
errors, but their noexcept specifiers turn these failures into
std::terminate.

Remove noexcept so the command handler can report the I/O error.
DispatchSockets() writes postponed_exception from its catch handler
without holding the input stream mutex. A synchronous reader can access
the same state after it is notified.

Acquire the mutex before storing the exception and notifying readers.
AsyncInputStream::Read() waits on caller_cond, but several asynchronous
EOF and error paths only notify the InputStream handler. Since
bd78307, the synchronous caller is not the installed handler and can
remain blocked forever.

Wrap InvokeOnAvailable() so every availability notification also wakes
caller_cond.
@acts-1631
acts-1631 force-pushed the fix/async-input-notification branch from 92e72d1 to 6785a54 Compare August 24, 2026 21:07
@acts-1631

Copy link
Copy Markdown
Contributor Author

Split these changes into separate documented commits. The noexcept removals are part of the reproduced failure path: after AsyncInputStream wakes the reader, NextSong() propagates the curl error, and noexcept would terminate MPD instead of allowing the command handler to report it. The ALSA mutex correction is now isolated in its own commit.

@MaxKellermann
MaxKellermann merged commit 405a344 into MusicPlayerDaemon:v0.24.x Aug 24, 2026
10 checks passed
@MaxKellermann

Copy link
Copy Markdown
Member

Much better, thanks.

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.

2 participants