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

Copy channels for public iterate() on internal queue. #919

Merged
merged 1 commit into from
Dec 3, 2019

Conversation

tcard
Copy link
Contributor

@tcard tcard commented Nov 26, 2019

We were getting the iterator in the internal queue, but then doing the
copy into public wrappers in the caller's queue. So the internal queue
could then concurrently run a task that mutates the collection while
it's being copied.

Now the copying is done while still on the internal queue, on the common
ARTChannels; Rest and Realtime just provide mappers to copy into the
right public channel type.

Fixes #918 (probably?).

We were getting the iterator in the internal queue, but then doing the
copy into public wrappers in the caller's queue. So the internal queue
could then concurrently run a task that mutates the collection while
it's being copied.

Now the copying is done while still on the internal queue, on the common
ARTChannels; Rest and Realtime just provide mappers to copy into the
right public channel type.

Fixes #918 (probably?).
@tcard tcard self-assigned this Nov 26, 2019
@tcard tcard requested a review from ricardopereira November 26, 2019 15:13
@tcard
Copy link
Contributor Author

tcard commented Nov 27, 2019

While #918 could (as far as I can tell) be caused by this too, the actual bug triggered by #916 is deterministic and is fixed by #920.

@tcard
Copy link
Contributor Author

tcard commented Dec 3, 2019

@QuintinWillison Anything outstanding here?

@QuintinWillison QuintinWillison added the bug Something isn't working. It's clear that this does need to be fixed. label Dec 3, 2019
@QuintinWillison QuintinWillison removed the request for review from ricardopereira December 3, 2019 10:59
@tcard tcard merged commit 23fa858 into develop Dec 3, 2019
@tcard tcard deleted the 918-channels-iterate-race branch December 3, 2019 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working. It's clear that this does need to be fixed.
Development

Successfully merging this pull request may close these issues.

Channels mutated-while-enumerated crash
2 participants