Skip to content

Provides more ByteBuf leaks fixes #803

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

Merged
merged 5 commits into from
Apr 27, 2020
Merged

Conversation

OlegDokuka
Copy link
Member

@OlegDokuka OlegDokuka commented Apr 27, 2020

This PR provides 4 central things:

  1. Ensures if there goes something during Payload to the frame we will not get any memory leaks in the end (first commit in the list)
  2. Fixes onDiscard leak which did not work correctly because actual subscriber was nulled to early
  3. Ensures that we will not have the wrong frame order in case of racing the first Payload sending and cancel. Also, it moves onDiscard hook to the very bottom in case of requestChannel to ensure the first payload is not leaked
  4. Enables a set of tests that ensures that NullPointerException on stream cancellation  #757 and ZERO_COPY and memory leaks  #733 are fully or partially fixed.

@OlegDokuka OlegDokuka requested a review from rstoyanchev April 27, 2020 18:54
@OlegDokuka OlegDokuka added the bug label Apr 27, 2020
@OlegDokuka OlegDokuka added this to the 1.0 RC7 milestone Apr 27, 2020
@OlegDokuka OlegDokuka linked an issue Apr 27, 2020 that may be closed by this pull request
@OlegDokuka OlegDokuka force-pushed the bugfix/leaks-followup branch from 7713697 to ca0bd35 Compare April 27, 2020 19:19
Copy link
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

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

Looks good. Nice catches!

Minor note, the ref count handling in the frames package looks a bit repetitive but we can tighten that after RC7.

@OlegDokuka OlegDokuka force-pushed the bugfix/leaks-followup branch from ca0bd35 to 9c32d2d Compare April 27, 2020 20:04
@OlegDokuka OlegDokuka changed the title Leaks fixups provides more ByteBuf leaks fixes Apr 27, 2020
@OlegDokuka OlegDokuka merged commit a5706bf into develop Apr 27, 2020
@OlegDokuka OlegDokuka deleted the bugfix/leaks-followup branch April 27, 2020 20:26
@OlegDokuka OlegDokuka changed the title provides more ByteBuf leaks fixes Provides more ByteBuf leaks fixes Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NullPointerException on stream cancellation
2 participants