Skip to content

Fix thread safety bug in split sender notify #1361

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.

Sign up for GitHub

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

Merged
ericniebler merged 1 commit into from
Jul 6, 2024
Merged

Fix thread safety bug in split sender notify #1361

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion include/stdexec/__detail/__shared.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,14 @@ namespace stdexec {
}

STDEXEC_ASSERT(__waiters_copy.front() != __get_tombstone());
for (__local_state_base* __item: __waiters_copy) {
for (auto __itr = __waiters_copy.begin(); __itr != __waiters_copy.end(); ) {
Copy link
Contributor Author

@ccotter ccotter Jul 5, 2024

Choose a reason for hiding this comment

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

I didn't find this fix all that satisfying (it seems a little too "cute"). Open to suggestions here. I considered changing the intrusive slist to handle this better, since the intrusive nature of the data structure leaves the iterator open to such bugs, but that didn't seem like the right thing to do.

__local_state_base* __item = *__itr;

// We must increment the iterator before calling notify, since notify
// may end up triggering *__item to be destructed on another thread,
// and the intrusive slist's iterator increment relies on __item.
++__itr;

__item->__notify_(__item);
}

Expand Down