Fix: Make sure to dup Array
in Channel.select_impl
#12827
Merged
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.
This patch fixes a bug in
Channel.select_impl
. The method duplicates the incoming collectionops
asops_locks
for sorting.ops
is anIndexable
and can be anArray
. Using#to_a
is insufficient for ensuring duplication becauseArray#to_a
returnsself
.As a result, the original
ops
would be modified through sorting if its anArray
. This is an unintended side-effect.Furthermore, the returned index which indicates the position of the action that was matched would correspond to the index in the sorted array and thus not correspond to the original indices.
This bug probably does not have a huge effect unless you're using the internal
Channel.select
API directly. Public uses are for the implementation of theselect
keyword, but that's using aTuple
type.Array
is typically used as a parameter toChannel.receive_first
andChannel.send_first
, though. Those methods do not return the index (ref #10411). The side-effect of mutating is also irrelevant because the list of actions is only created in the implementation of these methods from a list ofChannel
.For the latter use case, there is no need to duplicate the array. So I think we could introduce an optimization to avoid that, by extracting from
select_impl
a method which receives bothops
andops_locks
(which can the point to the same collection).