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.

Sign up for GitHub

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

Jump to bottom

ByteToMessageDecoder can pass a released buffer to PortUnificationServerHandler #1664

Closed
trustin opened this issue Jul 27, 2013 · 10 comments
Closed

ByteToMessageDecoder can pass a released buffer to PortUnificationServerHandler #1664

trustin opened this issue Jul 27, 2013 · 10 comments
Assignees
@trustin
Labels
defect
Milestone
4.0.6.Final

Comments

@trustin
Copy link
Member

trustin commented Jul 27, 2013 

io.netty.handler.codec.DecoderException: io.netty.util.IllegalReferenceCountException: refCnt: 0
    at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:253) ~[netty-perf-server-0.jar:na]
    at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:131) ~[netty-perf-server-0.jar:na]
    at io.netty.channel.DefaultChannelHandlerContext.invokeChannelRead(DefaultChannelHandlerContext.java:368) [netty-perf-server-0.jar:na]
    at io.netty.channel.DefaultChannelHandlerContext.fireChannelRead(DefaultChannelHandlerContext.java:353) [netty-perf-server-0.jar:na]
    at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:780) ~[netty-perf-server-0.jar:na]
    at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:100) ~[netty-perf-server-0.jar:na]
    at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:497) ~[netty-perf-server-0.jar:na]
    at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:465) ~[netty-perf-server-0.jar:na]
    at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:359) ~[netty-perf-server-0.jar:na]
    at io.netty.util.concurrent.SingleThreadEventExecutor$2.run(SingleThreadEventExecutor.java:101) ~[netty-perf-server-0.jar:na]
    at java.lang.Thread.run(Thread.java:724) ~[na:1.7.0_25]
Caused by: io.netty.util.IllegalReferenceCountException: refCnt: 0
    at io.netty.buffer.AbstractByteBuf.ensureAccessible(AbstractByteBuf.java:1171) ~[netty-perf-server-0.jar:na]
    at io.netty.buffer.AbstractByteBuf.checkIndex(AbstractByteBuf.java:1117) ~[netty-perf-server-0.jar:na]
    at io.netty.buffer.AbstractByteBuf.getByte(AbstractByteBuf.java:330) ~[netty-perf-server-0.jar:na]
    at io.netty.buffer.AbstractByteBuf.getUnsignedByte(AbstractByteBuf.java:343) ~[netty-perf-server-0.jar:na]
    at io.netty.handler.ssl.SslHandler.getEncryptedPacketLength(SslHandler.java:702) ~[netty-perf-server-0.jar:na]
    at io.netty.handler.ssl.SslHandler.isEncrypted(SslHandler.java:680) ~[netty-perf-server-0.jar:na]
    at com.twitter.netty.perf.server.PortUnificationServerHandler.isSsl(PortUnificationServerHandler.java:71) ~[netty-perf-server-0.jar:na]
    at com.twitter.netty.perf.server.PortUnificationServerHandler.decode(PortUnificationServerHandler.java:53) ~[netty-perf-server-0.jar:na]
    at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:231) ~[netty-perf-server-0.jar:na]

com.twitter.netty.perf.server.PortUnificationServerHandler is essentially identical with io.netty.example.portunification.PortUnificationServerHandler.
@trustin
Copy link
Member Author

trustin commented Jul 27, 2013

This is another reentrance issue. If a handler removes itself from the pipeline at decode(), callDecode() can pass the released (or even re-acquired) buffer to decode() again. We need to fix this fundamentally by making sure any handler methods are non-reentrant. We can implement it by maintaining some flag in DefaultChannelHandlerContext.

@trustin
Copy link
Member Author

trustin commented Jul 27, 2013

Actually, it has to be a variable (or one flag + one variable) rather than a flag so that we can check 1) if the invocation will be reentrant or not, and 2) if there are any pending tasks which were submitted to avoid the reentrance because otherwise the order of events can be wrong.

@normanmaurer
Copy link
Member

@trustin will you work on this?

Am 27.07.2013 um 10:01 schrieb Trustin Lee notifications@github.com:

This is another reentrance issue. If a handler removes itself from the pipeline at decode(), callDecode() can pass the released (or even re-acquired) buffer to decode() again. We need to fix this fundamentally by making sure any handler methods are non-reentrant. We can implement it by maintaining some flag in DefaultChannelHandlerContext.

—
Reply to this email directly or view it on GitHub.

@trustin
Copy link
Member Author

trustin commented Jul 27, 2013

Yeah.

@normanmaurer
Copy link
Member

Nice, thanks!

Am 27.07.2013 um 10:07 schrieb Trustin Lee notifications@github.com:

Yeah.

—
Reply to this email directly or view it on GitHub.

@normanmaurer
Copy link
Member

@trustin did you make any progress here ?

@trustin
Copy link
Member Author

trustin commented Jul 30, 2013

Nope because of other stuff at work.

Sent from a mobile device.
https://twitter.com/trustin
https://twitter.com/trustin_ko
https://twitter.com/netty_project

-----Original Message-----
From: Norman Maurer notifications@github.com
To: netty/netty netty@noreply.github.com
Cc: Trustin Lee t@motd.kr
Sent: Tue, 30 Jul 2013 1:52 PM
Subject: Re: [netty] ByteToMessageDecoder can pass a released buffer to PortUnificationServerHandler (#1664)

@trustin did you make any progress here ?

Reply to this email directly or view it on GitHub:
#1664 (comment)

@normanmaurer
Copy link
Member

@trustin: Should I take a stab on this ?

Am 30.07.2013 um 07:49 schrieb Trustin Lee notifications@github.com:

Nope because of other stuff at work.

Sent from a mobile device.
https://twitter.com/trustin
https://twitter.com/trustin_ko
https://twitter.com/netty_project

-----Original Message-----
From: Norman Maurer notifications@github.com
To: netty/netty netty@noreply.github.com
Cc: Trustin Lee t@motd.kr
Sent: Tue, 30 Jul 2013 1:52 PM
Subject: Re: [netty] ByteToMessageDecoder can pass a released buffer to PortUnificationServerHandler (#1664)

@trustin did you make any progress here ?

Reply to this email directly or view it on GitHub:
#1664 (comment)
—
Reply to this email directly or view it on GitHub.

@ghost ghost assigned trustin Jul 31, 2013
@trustin
Copy link
Member Author

trustin commented Jul 31, 2013

This specific issue has been fixed. Will think about eliminating reentrance / overlapped invocation completely later.

normanmaurer pushed a commit that referenced this issue Aug 1, 2013
Correctly fix problem in ByteToMessageDecoder and ReplayingDecoder wh…
a06295f 
…ich could let to have a released buffer passed to the decode methods.

This fixes #1664 and revert also the original commit which was meant to fix it 3b1881b . The problem with the original commit was that it could delay handlerRemove(..) calls and so mess up the order or forward bytes to late.
@lucwillems
Copy link

Hi all ,
we seem to have a similar issue with a Datagram based channel
ERROR|el-dc-1|2014-11-20 13:40:11.053|com.twentyonenet.tunnel.handler.DatagramMarChannelResolvingHandler.exceptionCaught(31)|uncaught exception:
io.netty.handler.codec.DecoderException: io.netty.util.IllegalReferenceCountException: refCnt: 0
at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:144)
at io.netty.handler.codec.ByteToMessageCodec.channelRead(ByteToMessageCodec.java:108)
at io.netty.channel.ChannelHandlerInvokerUtil.invokeChannelReadNow(ChannelHandlerInvokerUtil.java:74)
at io.netty.channel.DefaultChannelHandlerInvoker.invokeChannelRead(DefaultChannelHandlerInvoker.java:138)
at io.netty.channel.DefaultChannelHandlerContext.fireChannelRead(DefaultChannelHandlerContext.java:320)
at com.twentyonenet.tunnel.handler.DatagramHandler.channelRead(DatagramHandler.java:41)
at io.netty.channel.ChannelHandlerInvokerUtil.invokeChannelReadNow(ChannelHandlerInvokerUtil.java:74)
at io.netty.channel.DefaultChannelHandlerInvoker.invokeChannelRead(DefaultChannelHandlerInvoker.java:138)
at io.netty.channel.DefaultChannelHandlerContext.fireChannelRead(DefaultChannelHandlerContext.java:320)
at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:846)
at io.netty.channel.nio.AbstractNioMessageChannel$NioMessageUnsafe.read(AbstractNioMessageChannel.java:92)
at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:485)
at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:452)
at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:346)
at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:794)
at java.lang.Thread.run(Thread.java:744)
Caused by: io.netty.util.IllegalReferenceCountException: refCnt: 0
at io.netty.buffer.AbstractByteBuf.ensureAccessible(AbstractByteBuf.java:1173)
at io.netty.buffer.PooledByteBuf.capacity(PooledByteBuf.java:77)
at io.netty.buffer.AbstractByteBuf.ensureWritable(AbstractByteBuf.java:251)
at io.netty.buffer.AbstractByteBuf.writeBytes(AbstractByteBuf.java:842)
at io.netty.buffer.AbstractByteBuf.writeBytes(AbstractByteBuf.java:835)
at io.netty.buffer.AbstractByteBuf.writeBytes(AbstractByteBuf.java:825)
at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:137)
... 15 more

this is using netty 5.0.0-ALPHA1 , sending 300Mbit/sec of UDP data messages (full 1500 bytes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@trustin trustin

Labels
defect
Projects
None yet
Milestone
4.0.6.Final
Development

No branches or pull requests
3 participants
@trustin @normanmaurer @lucwillems