Skip to content

Conversation

@violetagg
Copy link
Member

This change handles the use case below:
When there is the sequence

    .aggregate()
    .asString()

If a timeout happens while in aggregate() the composite byte buffer
will be released. Later when asString() is invoked
IllegalReferenceCountException will be throws:

io.netty.util.IllegalReferenceCountException: refCnt: 0
    at io.netty.buffer.AbstractByteBuf.ensureAccessible(AbstractByteBuf.java:1417)
    at io.netty.buffer.AbstractByteBuf.checkIndex(AbstractByteBuf.java:1356)
    at io.netty.buffer.AbstractByteBuf.checkDstIndex(AbstractByteBuf.java:1376)
    at io.netty.buffer.CompositeByteBuf.getBytes(CompositeByteBuf.java:854)
    at io.netty.buffer.CompositeByteBuf.getBytes(CompositeByteBuf.java:44)
    at io.netty.buffer.PooledHeapByteBuf.setBytes(PooledHeapByteBuf.java:230)
    at io.netty.buffer.AbstractByteBuf.writeBytes(AbstractByteBuf.java:1080)
    at io.netty.buffer.ByteBufUtil.decodeString(ByteBufUtil.java:781)
    at io.netty.buffer.AbstractByteBuf.toString(AbstractByteBuf.java:1222)
    at io.netty.buffer.AbstractByteBuf.toString(AbstractByteBuf.java:1217)
    at reactor.netty.ByteBufMono.lambda$asString$1(ByteBufMono.java:78)

Related to #422

@violetagg violetagg added this to the 0.7.11.RELEASE milestone Oct 19, 2018
@violetagg violetagg requested a review from smaldini October 19, 2018 20:15
@codecov-io
Copy link

codecov-io commented Oct 19, 2018

Codecov Report

Merging #470 into 0.7.x will decrease coverage by 0.15%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##              0.7.x     #470      +/-   ##
============================================
- Coverage     68.88%   68.73%   -0.16%     
+ Complexity     1076     1075       -1     
============================================
  Files            73       73              
  Lines          4544     4554      +10     
  Branches        660      660              
============================================
  Hits           3130     3130              
- Misses         1031     1042      +11     
+ Partials        383      382       -1
Impacted Files Coverage Δ Complexity Δ
src/main/java/reactor/ipc/netty/ByteBufFlux.java 75.47% <66.66%> (-1.62%) 22 <2> (+1)
src/main/java/reactor/ipc/netty/ByteBufMono.java 48.14% <66.66%> (+2.69%) 7 <2> (+1) ⬆️
.../ipc/netty/channel/PooledClientContextHandler.java 66.38% <0%> (-6.73%) 26% <0%> (-3%)
...java/reactor/ipc/netty/channel/ContextHandler.java 74.75% <0%> (-2.92%) 27% <0%> (-1%)
...a/reactor/ipc/netty/channel/ChannelOperations.java 82.67% <0%> (-1.58%) 48% <0%> (-1%)
...or/ipc/netty/channel/ChannelOperationsHandler.java 66.01% <0%> (+0.87%) 61% <0%> (+1%) ⬆️
...ctor/ipc/netty/resources/DefaultPoolResources.java 79.59% <0%> (+3.06%) 11% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fff576...8d0c7a4. Read the comment docs.

…hods

This change handles the use case below:
When there is the sequence
    .aggregate()
    .asString()

If a timeout happens while in `aggregate()` the composite byte buffer
will be released. Later when `asString()` is invoked
IllegalReferenceCountException will be throws:

```
io.netty.util.IllegalReferenceCountException: refCnt: 0
    at io.netty.buffer.AbstractByteBuf.ensureAccessible(AbstractByteBuf.java:1417)
    at io.netty.buffer.AbstractByteBuf.checkIndex(AbstractByteBuf.java:1356)
    at io.netty.buffer.AbstractByteBuf.checkDstIndex(AbstractByteBuf.java:1376)
    at io.netty.buffer.CompositeByteBuf.getBytes(CompositeByteBuf.java:854)
    at io.netty.buffer.CompositeByteBuf.getBytes(CompositeByteBuf.java:44)
    at io.netty.buffer.PooledHeapByteBuf.setBytes(PooledHeapByteBuf.java:230)
    at io.netty.buffer.AbstractByteBuf.writeBytes(AbstractByteBuf.java:1080)
    at io.netty.buffer.ByteBufUtil.decodeString(ByteBufUtil.java:781)
    at io.netty.buffer.AbstractByteBuf.toString(AbstractByteBuf.java:1222)
    at io.netty.buffer.AbstractByteBuf.toString(AbstractByteBuf.java:1217)
    at reactor.netty.ByteBufMono.lambda$asString$1(ByteBufMono.java:78)
```

Related to #422
@violetagg violetagg force-pushed the handle-ref-exception-on-cancellation branch from 3de0d0c to 8d0c7a4 Compare October 19, 2018 21:06
@violetagg violetagg changed the title Handle IllegalReferenceCountException in ByteBuf(Flux|Mono)#asString Handle IllegalReferenceCountException in ByteBuf(Flux|Mono)#as... methods Oct 19, 2018
@violetagg violetagg merged commit 8d0c7a4 into 0.7.x Oct 19, 2018
@violetagg violetagg deleted the handle-ref-exception-on-cancellation branch October 19, 2018 21:12
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.

4 participants