Skip to content

Fix wrong usage of GetScratchBuffer - #2101

Merged
snnn merged 1 commit into
masterfrom
zhalei/fix_wrong_GetScratchBuffer_usage_usage
Oct 18, 2019
Merged

Fix wrong usage of GetScratchBuffer#2101
snnn merged 1 commit into
masterfrom
zhalei/fix_wrong_GetScratchBuffer_usage_usage

Conversation

@zhanghuanrong

Copy link
Copy Markdown
Contributor

the error pattern looks like:
auto p = GetScratchBuffer(size).get();
which cause cuda memory freed immediately, but p will be used later.
This also cause random error.

  auto p = GetScratchBuffer<T>(size).get();
which cause cuda memory freed immediately, but p will be used later.
This also cause random error.
@zhanghuanrong
zhanghuanrong requested a review from a team as a code owner October 11, 2019 19:23

auto condition_cumulative_sum = GetScratchBuffer<int32_t>(valid_condition_length).get();
auto condition_cumulative_sum_buffer = GetScratchBuffer<int32_t>(valid_condition_length);
auto condition_cumulative_sum = condition_cumulative_sum_buffer.get();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

condition_cumulative_sum_buffer [](start = 34, length = 31)

Thanks for the fix. Do we know why it was not caught by tests? How can we prevent future errors like this?

@ke1337
ke1337 requested a review from a team October 11, 2019 20:51
@snnn
snnn merged commit 47bc351 into master Oct 18, 2019
@snnn
snnn deleted the zhalei/fix_wrong_GetScratchBuffer_usage_usage branch October 18, 2019 15:15
yuslepukhin pushed a commit that referenced this pull request Mar 17, 2026
## Describe your changes
- Not used at all and maintained. 
- Same as #2101 but additional files removed

## Checklist before requesting a review
- [ ] Add unit tests for this change.
- [ ] Make sure all tests can pass.
- [ ] Update documents if necessary.
- [ ] Lint and apply fixes to your code by running `lintrunner -a`
- [ ] Is this a user-facing change? If yes, give a description of this
change to be included in the release notes.
- [ ] Is this PR including examples changes? If yes, please remember to
update [example
documentation](https://github.com/microsoft/Olive/blob/main/docs/source/examples.md)
in a follow-up PR.

## (Optional) Issue link
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.

5 participants