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

[#116] Fix retrieve buffer overflow #117

Merged

Conversation

elfenpiff
Copy link
Contributor

@elfenpiff elfenpiff commented Feb 17, 2024

Notes for Reviewer

The retrieve buffer can overflow when the publisher loans many samples and send them one by one without any more loans. The problem is, that the samples in the retrieve buffer are only recollected whenever a new loan is acquired. When we never have to call loan again since we have a big buffer of loaned samples on the publisher side, the retrieve buffer will overflow and the communication fails.

The solution is rather simple: Call retrieve_returned_samples before sending any new samples. In this way, we ensure that we reclaim all borrowed samples before we borrow (send) new samples to the receiver.

Pre-Review Checklist for the PR Author

  1. Add sensible notes for the reviewer
  2. PR title is short, expressive and meaningful
  3. Relevant issues are linked
  4. Every source code file has a copyright header with SPDX-License-Identifier: Apache-2.0 OR MIT
  5. Branch follows the naming format (iox2-123-introduce-posix-ipc-example)
  6. Commits messages are according to this guideline
  7. Tests follow the best practice for testing
  8. Changelog updated in the unreleased section including API breaking changes
  9. Assign PR to reviewer
  10. All checks have passed (except task-list-completed)

Checklist for the PR Reviewer

  • Commits are properly organized and messages are according to the guideline
  • Unit tests have been written for new behavior
  • Public API is documented
  • PR title describes the changes

Post-review Checklist for the PR Author

  1. All open points are addressed and tracked via issues

References

Use either 'Closes #123' or 'Relates to #123' to reference the corresponding issue.

Relates to #116

@elfenpiff elfenpiff requested a review from elBoberido February 17, 2024 16:33
@elfenpiff elfenpiff changed the title [#116] Fix retrieve buffer overflow [DO NOT REVIEW, based on PR #99] [#116] Fix retrieve buffer overflow Feb 17, 2024
@elfenpiff elfenpiff self-assigned this Feb 17, 2024
Copy link

codecov bot commented Feb 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (dfd44b1) 77.36% compared to head (dcdc80f) 77.36%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #117   +/-   ##
=======================================
  Coverage   77.36%   77.36%           
=======================================
  Files         181      181           
  Lines       19596    19598    +2     
=======================================
+ Hits        15160    15162    +2     
  Misses       4436     4436           
Files Coverage Δ
iceoryx2/src/port/publisher.rs 83.33% <100.00%> (+0.10%) ⬆️

@elfenpiff elfenpiff changed the title [DO NOT REVIEW, based on PR #99] [#116] Fix retrieve buffer overflow [#116] Fix retrieve buffer overflow Feb 17, 2024
@elfenpiff elfenpiff force-pushed the iox2-116-fix-retrieve-buffer-overflow branch from 9c5b64e to c245344 Compare February 17, 2024 17:10
@elfenpiff elfenpiff requested a review from elBoberido February 18, 2024 09:44
@elfenpiff elfenpiff force-pushed the iox2-116-fix-retrieve-buffer-overflow branch from c68f623 to dcdc80f Compare February 18, 2024 09:46
@elfenpiff elfenpiff merged commit 485b15d into eclipse-iceoryx:main Feb 18, 2024
23 checks passed
@elfenpiff elfenpiff deleted the iox2-116-fix-retrieve-buffer-overflow branch February 18, 2024 14:37
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.

2 participants