Skip to content

Fix use two-iterator erase-remove in multiplexer::cancel to avoid UB - #428

Merged
anarthal merged 1 commit into
boostorg:developfrom
L0rentz:fix/multiplexer-cancel-erase-remove
Jun 26, 2026
Merged

Fix use two-iterator erase-remove in multiplexer::cancel to avoid UB#428
anarthal merged 1 commit into
boostorg:developfrom
L0rentz:fix/multiplexer-cancel-erase-remove

Conversation

@L0rentz

@L0rentz L0rentz commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix an incorrect erase-remove use in multiplexer::cancel.

The current code calls the single-iterator overload of deque::erase on the result of std::remove:

reqs_.erase(std::remove(std::begin(reqs_), std::end(reqs_), ptr));

If ptr is not present in reqs_, std::remove returns end(), making this erase(end()), which is undefined behavior.

This can happen when a request is cancelled after it has already been removed from the queue, for example during connection teardown or reconnect handling.

The fix is to use the normal two-iterator erase-remove form:

reqs_.erase(std::remove(std::begin(reqs_), std::end(reqs_), ptr), std::end(reqs_));

This also matches the two-iterator erase(first, last) form used by the other queue-trimming functions in this file (cancel_waiting, cancel_on_conn_lost, release_push_requests).

Testing

Observed with -D_GLIBCXX_DEBUG, where the invalid erase(end()) path aborts with a past-the-end iterator error.

@cppalliance-bot

Copy link
Copy Markdown

An automated preview of the documentation is available at https://428.redis.prtest3.cppalliance.org/libs/redis/doc/html/index.html

If more commits are pushed to the pull request, the docs will rebuild at the same URL.

2026-06-17 06:08:24 UTC

@anarthal

Copy link
Copy Markdown
Collaborator

Thanks. I think we can merge this once CI passes. I'll try to add a regression test later.

@anarthal

Copy link
Copy Markdown
Collaborator

Merging as #430 should fix the only CI error.

@anarthal
anarthal merged commit 741d389 into boostorg:develop Jun 26, 2026
78 of 80 checks passed
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.

3 participants