Skip to content

Commit

Permalink
Move resetting the composite reader index to before the release as it…
Browse files Browse the repository at this point in the history
… that fails then the operation hasn't completed correctly and the ownership of the buffer should remain with the caller.
  • Loading branch information
larry-safran committed Sep 6, 2023
1 parent 80df75d commit 426f1e6
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,8 @@ static void mergeWithCompositeTail(
composite.addFlattenedComponents(true, newTail);
// New tail's ownership transferred to the composite buf.
newTail = null;
in.release();
in = null;
// Restore the reader. In case it fails we restore the reader after releasing/forgetting
// the input and the new tail so that finally block can handles them properly.
composite.readerIndex(prevReader);
in.release(); // Successful so took over ownership of in
} finally {
// If new tail's ownership isn't transferred to the composite buf.
// Release it to prevent a leak.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ public CompositeByteBuf addFlattenedComponents(boolean increaseWriterIndex,
fail("Cumulator didn't throw");
} catch (UnsupportedOperationException actualError) {
assertSame(expectedError, actualError);
// Input must be released unless its ownership has been to the composite cumulation.
// Because of error, ownership shouldn't have changed so should not have been released.
assertEquals(1, in.refCnt());
// Tail released
assertEquals(0, tail.refCnt());
Expand Down Expand Up @@ -573,7 +573,7 @@ public CompositeByteBuf addFlattenedComponents(boolean increaseWriterIndex,
fail("Cumulator didn't throw");
} catch (UnsupportedOperationException actualError) {
assertSame(expectedError, actualError);
// Input must be released unless its ownership has been to the composite cumulation.
// Because of error, ownership shouldn't have changed so should not have been released.
assertEquals(1, in.refCnt());
// New buffer released
assertEquals(0, newTail.refCnt());
Expand Down

0 comments on commit 426f1e6

Please sign in to comment.