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.

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

4.1.9 breaks epoll selector rebuild #6607

Closed
nhw76 opened this issue Apr 5, 2017 · 8 comments

Comments

Projects
None yet
4 participants
@nhw76
Copy link

commented Apr 5, 2017

The following commit appears to break the epoll selector rebuild logic in rebuildSelector():

795f318

If the logic is fired by an excessive number of premature returns, you will hit an exception like this:

java.lang.ClassCastException: io.netty.channel.nio.SelectedSelectionKeySetSelector cannot be cast to java.nio.channels.spi.AbstractSelector
        at java.nio.channels.spi.AbstractSelectableChannel.register(AbstractSelectableChannel.java:212)
        at io.netty.channel.nio.NioEventLoop.rebuildSelector0(NioEventLoop.java:347)
        at io.netty.channel.nio.NioEventLoop.rebuildSelector(NioEventLoop.java:318)
        at io.netty.channel.nio.NioEventLoop.select(NioEventLoop.java:769)
        at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:388)
        at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:858)
        at java.lang.Thread.run(Thread.java:745)

Looks like a case of replacing the selector member with the unwrappedSelector() method may have been missed?

@Scottmitch Scottmitch self-assigned this Apr 5, 2017

@Scottmitch Scottmitch added the defect label Apr 5, 2017

@Scottmitch Scottmitch added this to the 4.1.10.Final milestone Apr 5, 2017

Scottmitch added a commit to Scottmitch/netty that referenced this issue Apr 5, 2017

NioEventLoop#rebuildSelector0 throws ClassCastException
Motivation:
Commit 795f318 simplified some code related to the special case Set for the selected keys and introduced a Selector wrapper to make sure this set was properly reset. However the JDK makes assumptions about the type of Selector and this type is not extensible. This means whenever we call into the JDK we must provide the unwrapped version of the Selector or we get a ClassCastException. We missed a case of unwrapping in NioEventLoop#rebuildSelector0.

Modificaitons:
- NioEventLoop#openSelector should return a tuple so we can atomically set the wrapped and unwrapped Selector
- NioEventLoop#rebuildSelector0 should use the unwrapped version of the selector

Result:
Fixes netty#6607.
@Scottmitch

This comment has been minimized.

Copy link
Member

commented Apr 5, 2017

see #6608

@Scottmitch Scottmitch closed this in c37267d Apr 6, 2017

Scottmitch added a commit that referenced this issue Apr 6, 2017

NioEventLoop#rebuildSelector0 throws ClassCastException
Motivation:
Commit 795f318 simplified some code related to the special case Set for the selected keys and introduced a Selector wrapper to make sure this set was properly reset. However the JDK makes assumptions about the type of Selector and this type is not extensible. This means whenever we call into the JDK we must provide the unwrapped version of the Selector or we get a ClassCastException. We missed a case of unwrapping in NioEventLoop#rebuildSelector0.

Modificaitons:
- NioEventLoop#openSelector should return a tuple so we can atomically set the wrapped and unwrapped Selector
- NioEventLoop#rebuildSelector0 should use the unwrapped version of the selector

Result:
Fixes #6607.
@Scottmitch

This comment has been minimized.

Copy link
Member

commented Apr 6, 2017

please re-open if PR #6608 didn't fix the issue

@md-5

This comment has been minimized.

Copy link
Contributor

commented Jul 14, 2017

https://hub.spigotmc.org/jira/browse/SPIGOT-3418
User is suggesting this issue is there in 4.1.13

@normanmaurer

This comment has been minimized.

Copy link
Member

commented Jul 15, 2017

Let me check

normanmaurer added a commit that referenced this issue Jul 15, 2017

Add testcase to ensure NioEventLoop.rebuildSelector() works correctly.
Motivation:

We had recently a report that the issue [#6607] is still not fixed.

Modifications:

Add a testcase to prove the issue is fuxed.

Result:

More tests.
@normanmaurer

This comment has been minimized.

Copy link
Member

commented Jul 15, 2017

@md-5 it is fixed in 4.1.13 and to prove this I added a unit-test. Please check:
#6971

normanmaurer added a commit that referenced this issue Jul 15, 2017

Add testcase to ensure NioEventLoop.rebuildSelector() works correctly.
Motivation:

We had recently a report that the issue [#6607] is still not fixed.

Modifications:

Add a testcase to prove the issue is fuxed.

Result:

More tests.
@md-5

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2017

Hmm thanks for looking, this is strange.
It is an isolated report so far, I can only think maybe the user ended up with a different Netty version than the one we bundled.

@md-5

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2017

Apologies @normanmaurer , looks like a packaging error on our behalf caused the incorrect version to be used.

Thanks for looking anyway

normanmaurer added a commit that referenced this issue Jul 16, 2017

Add testcase to ensure NioEventLoop.rebuildSelector() works correctly.
Motivation:

We had recently a report that the issue [#6607] is still not fixed.

Modifications:

Add a testcase to prove the issue is fuxed.

Result:

More tests.
@normanmaurer

This comment has been minimized.

Copy link
Member

commented Jul 16, 2017

normanmaurer added a commit that referenced this issue Jul 17, 2017

Add testcase to ensure NioEventLoop.rebuildSelector() works correctly.
Motivation:

We had recently a report that the issue [#6607] is still not fixed.

Modifications:

Add a testcase to prove the issue is fuxed.

Result:

More tests.

normanmaurer added a commit that referenced this issue Jul 18, 2017

Add testcase to ensure NioEventLoop.rebuildSelector() works correctly.
Motivation:

We had recently a report that the issue [#6607] is still not fixed.

Modifications:

Add a testcase to prove the issue is fixed.

Result:

More tests.

normanmaurer added a commit that referenced this issue Jul 18, 2017

Add testcase to ensure NioEventLoop.rebuildSelector() works correctly.
Motivation:

We had recently a report that the issue [#6607] is still not fixed.

Modifications:

Add a testcase to prove the issue is fixed.

Result:

More tests.

normanmaurer added a commit that referenced this issue Jul 18, 2017

Add testcase to ensure NioEventLoop.rebuildSelector() works correctly.
Motivation:

We had recently a report that the issue [#6607] is still not fixed.

Modifications:

Add a testcase to prove the issue is fixed.

Result:

More tests.

liuzhengyang pushed a commit to liuzhengyang/netty that referenced this issue Sep 10, 2017

NioEventLoop#rebuildSelector0 throws ClassCastException
Motivation:
Commit 795f318 simplified some code related to the special case Set for the selected keys and introduced a Selector wrapper to make sure this set was properly reset. However the JDK makes assumptions about the type of Selector and this type is not extensible. This means whenever we call into the JDK we must provide the unwrapped version of the Selector or we get a ClassCastException. We missed a case of unwrapping in NioEventLoop#rebuildSelector0.

Modificaitons:
- NioEventLoop#openSelector should return a tuple so we can atomically set the wrapped and unwrapped Selector
- NioEventLoop#rebuildSelector0 should use the unwrapped version of the selector

Result:
Fixes netty#6607.

liuzhengyang pushed a commit to liuzhengyang/netty that referenced this issue Sep 10, 2017

Add testcase to ensure NioEventLoop.rebuildSelector() works correctly.
Motivation:

We had recently a report that the issue [netty#6607] is still not fixed.

Modifications:

Add a testcase to prove the issue is fixed.

Result:

More tests.

kiril-me added a commit to kiril-me/netty that referenced this issue Feb 28, 2018

NioEventLoop#rebuildSelector0 throws ClassCastException
Motivation:
Commit 795f318 simplified some code related to the special case Set for the selected keys and introduced a Selector wrapper to make sure this set was properly reset. However the JDK makes assumptions about the type of Selector and this type is not extensible. This means whenever we call into the JDK we must provide the unwrapped version of the Selector or we get a ClassCastException. We missed a case of unwrapping in NioEventLoop#rebuildSelector0.

Modificaitons:
- NioEventLoop#openSelector should return a tuple so we can atomically set the wrapped and unwrapped Selector
- NioEventLoop#rebuildSelector0 should use the unwrapped version of the selector

Result:
Fixes netty#6607.

kiril-me added a commit to kiril-me/netty that referenced this issue Feb 28, 2018

Add testcase to ensure NioEventLoop.rebuildSelector() works correctly.
Motivation:

We had recently a report that the issue [netty#6607] is still not fixed.

Modifications:

Add a testcase to prove the issue is fixed.

Result:

More tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.