[#4435] Always invoke the actual deregisteration later in the EventLoop. #4621
Conversation
|
@trustin @Scottmitch PTAL |
|
@trustin @Scottmitch PTAL... Fix was verified by @doom369 who reported the issue before. |
| invokeLater(new OneTimeTask() { | ||
| @Override | ||
| public void run() { | ||
| pipeline.fireChannelUnregistered(); |
There was a problem hiding this comment.
We are already in OneTimeTask. Perhaps we could call pipeline.fireChannelUnregistered() directly?
There was a problem hiding this comment.
@trustin I thought it was "delayed" for some reason in the past but could not really remember, so I thought I should just keep it delayed. But if you think it is fine to directly call the method after pipeline.fireChannelInactive() I'm happy to do so. WDYT ?
There was a problem hiding this comment.
We are already in invokeLater, so I don't think we need to do it again. i.e. direct call should be OK IMO.
There was a problem hiding this comment.
alright... let me update the PR and see if we get any issues :)
Motivation: As a user may call deregister() from within any method while doing processing in the ChannelPipeline, we need to ensure we do the actual deregister operation later. This is needed as for example, we may be in the ByteToMessageDecoder.callDecode(...) method and so still try to do processing in the old EventLoop while the user already registered the Channel to a new EventLoop. Without delay, the deregister operation this could lead to have a handler invoked by different EventLoop and so threads. Modifications: Ensure the actual deregister will be done later on and not directly when invoked. Result: Calling deregister() within ByteToMessageDecoder.decode(..) is safe.
normanmaurer
force-pushed
the
invoke_later_deregister
branch
from
cc1880a
to
8283e4d
Compare
|
lgtm ... good find! |
Motivation:
As a user may call deregister() from within any method while doing processing in the ChannelPipeline, we need to ensure we do the actual deregister operation later. This is needed as for example, we may be in the ByteToMessageDecoder.callDecode(...) method and so still try to do processing in the old EventLoop while the user already registered the Channel to a new EventLoop. Without delay, the deregister operation this could lead to have a handler invoked by different EventLoop and so threads.
Modifications:
Ensure the actual deregister will be done later on and not directly when invoked.
Result:
Calling deregister() within ByteToMessageDecoder.decode(..) is safe.