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

[BUG] Potentially undefined behavior of cuda_stream_pool::get_stream #871

Closed
achirkin opened this issue Sep 15, 2021 · 1 comment · Fixed by #873
Closed

[BUG] Potentially undefined behavior of cuda_stream_pool::get_stream #871

achirkin opened this issue Sep 15, 2021 · 1 comment · Fixed by #873
Labels
bug Something isn't working

Comments

@achirkin
Copy link

These functions seem to have the undefined behavior (remainder of zero division) for the case when the pool size is zero (which is the default in raft::handle_t and as such is rather dangerous):

rmm::cuda_stream_view get_stream() const noexcept
{
return streams_[(next_stream++) % streams_.size()].view();
}

rmm::cuda_stream_view get_stream(std::size_t stream_id) const
{
return streams_[stream_id % streams_.size()].view();
}

@achirkin achirkin changed the title Potentially undefined behavior of cuda_stream_pool::get_stream [BUG] Potentially undefined behavior of cuda_stream_pool::get_stream Sep 15, 2021
@jrhemstad
Copy link
Contributor

Hm, we should probably disallow constructing with a pool size of 0. I can't think of a sane behavior of get_stream when the pool is empty.

@harrism harrism added the bug Something isn't working label Sep 16, 2021
@rapids-bot rapids-bot bot closed this as completed in #873 Sep 17, 2021
rapids-bot bot pushed a commit that referenced this issue Sep 17, 2021
An exception is now thrown when zero is passed as the size parameter to the `cuda_stream_pool` constructor. Also adds a test for this exception.

Fixes #871

Authors:
  - Mark Harris (https://github.com/harrism)

Approvers:
  - Jake Hemstad (https://github.com/jrhemstad)
  - Rong Ou (https://github.com/rongou)

URL: #873
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants