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

Implementation of the latest stable Netty4 version #902

Merged
merged 46 commits into from May 30, 2020
Merged

Implementation of the latest stable Netty4 version #902

merged 46 commits into from May 30, 2020

Conversation

valib
Copy link
Contributor

@valib valib commented May 26, 2016

@UniversalMediaServer/developers can we test the latest Netty 4 version if it is worth to be used? For me it works.


This change is Reviewable

@valib valib closed this May 26, 2016
@valib valib reopened this May 26, 2016
@SubJunk
Copy link
Member

SubJunk commented May 27, 2016

@valib cool, maybe we can release a beta version with this update to see how it works for people. Netty 4 always worked perfectly for me but others had issues, so I'm not able to test it reliably.

@Sami32
Copy link
Contributor

Sami32 commented May 31, 2016

@valib I was just thinking about that with the 5.2.1 version to see if freezing/timeouts will be here again...maybe not pertinent but hope doesn't kill anyone :-D
I will be happy to test it.

EDIT : i tested already and didn't get any streaming problems or errors in my LOG.
So maybe @skeptical with his chunk renderer can make more testing, because i guess you did already with Sony TV and PlayStation.

@SubJunk
Copy link
Member

SubJunk commented Jun 17, 2016

This is working well for me. Maybe we can release it as a 7.0.0 beta soon, what do you think?

@Sami32
Copy link
Contributor

Sami32 commented Jun 18, 2016

@valib @SubJunk Look promising :-)

I meeted a warning in the LOG though, but i will do more testing this week-end.

WARN 2016-06-18 02:46:19.203 [main] Failed to find a usable hardware address from the network interfaces; using random bytes: 22:e9:f9:4f:86:54:0c:8d
DEBUG 2016-06-18 02:46:19.203 [main] -Dio.netty.machineId: 22:e9:f9:4f:86:54:0c:8d (auto-detected)

UMS-debug_Netty4_Failed.txt

EDIT : Not sure it's pertinent, but just wondering if this could affect positively #899 and #920 ?

AsciiString is a new CharSequence implementation which contains only 1-byte characters. You will find this class useful when you deal with a US-ASCII or ISO-8859-1 string.
For instance, the HTTP codec and STOMP codec in Netty use AsciiString to represent the header names. Because AsciiString does not have any conversion cost when encoding it into a ByteBuf, it guarantees better performance than using String.

EDIT 2 : Before it was UTF8 encoding.

INFO 2016-06-18 13:45:25.500 [main] Encoding: Cp1252
...
DEBUG 2016-06-18 14:09:16.109 [cling-53] Error parsing xml: com.sun.org.apache.xerces.internal.impl.io.MalformedByteSequenceException: Invalid byte 2 of 3-byte UTF-8 sequence.

UMS-debug_Netty4.1.1.txt.zip.txt

P.S. My bad :/ i was running directly from depandencies JAR snapshot, so -Dfile.encoding=utf-8 parameters was missing i guess.

…iaServer into Netty4

# Conflicts:
#	src/main/java/net/pms/network/UPNPHelper.java
@valib
Copy link
Contributor Author

valib commented Oct 29, 2016

Rebased + merged to the latest master.

Conflicts:
	pom.xml
	src/main/java/net/pms/network/HTTPServer.java
	src/main/java/net/pms/network/RequestHandlerV2.java
	src/main/java/net/pms/network/RequestV2.java
	src/main/java/net/pms/network/UPNPControl.java
Conflicts:
	src/main/java/net/pms/network/RequestHandlerV2.java
	src/main/java/net/pms/network/RequestV2.java
@valib
Copy link
Contributor Author

valib commented Feb 13, 2020

@SubJunk now it is ready for testing

@valib valib added this to Review in progress in Universal Media Server May 28, 2020
@SubJunk
Copy link
Member

SubJunk commented May 28, 2020

@valib I'm interested in getting this merged. I have tested it and it seems good to me. I am thinking of maybe separating it from v10, because it might make it harder to identify bugs in the API code, so what do you think about it going out as 9.6.0?

@valib
Copy link
Contributor Author

valib commented May 28, 2020

@SubJunk agree with you to release it in the 9.6.0

Copy link
Member

@SubJunk SubJunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of comments before merge @valib

src/main/java/net/pms/network/RequestHandlerV2.java Outdated Show resolved Hide resolved
Co-authored-by: SubJunk <SubJunk@users.noreply.github.com>
@SubJunk SubJunk merged commit 2e1bfa9 into master May 30, 2020
Universal Media Server automation moved this from Review in progress to Done May 30, 2020
@SubJunk SubJunk deleted the Netty4 branch May 30, 2020 09:17
@SubJunk
Copy link
Member

SubJunk commented Jun 5, 2020

@valib I can see some exceptions in the logs after this merge. I haven't noticed any actual bugs so it may not be important but here they are:

[main] direct buffer constructor: unavailable
java.lang.UnsupportedOperationException: Reflective setAccessible(true) disabled
	at io.netty.util.internal.ReflectionUtil.trySetAccessible(ReflectionUtil.java:31)
	at io.netty.util.internal.PlatformDependent0$4.run(PlatformDependent0.java:233)
	at java.base/java.security.AccessController.doPrivileged(Unknown Source)
	at io.netty.util.internal.PlatformDependent0.<clinit>(PlatformDependent0.java:227)
	at io.netty.util.internal.PlatformDependent.isAndroid(PlatformDependent.java:289)
	at io.netty.util.internal.PlatformDependent.<clinit>(PlatformDependent.java:92)
	at io.netty.channel.nio.NioEventLoop.newTaskQueue0(NioEventLoop.java:279)
	at io.netty.channel.nio.NioEventLoop.newTaskQueue(NioEventLoop.java:150)
	at io.netty.channel.nio.NioEventLoop.<init>(NioEventLoop.java:138)
	at io.netty.channel.nio.NioEventLoopGroup.newChild(NioEventLoopGroup.java:146)
	at io.netty.channel.nio.NioEventLoopGroup.newChild(NioEventLoopGroup.java:37)
	at io.netty.util.concurrent.MultithreadEventExecutorGroup.<init>(MultithreadEventExecutorGroup.java:84)
	at io.netty.util.concurrent.MultithreadEventExecutorGroup.<init>(MultithreadEventExecutorGroup.java:58)
	at io.netty.channel.MultithreadEventLoopGroup.<init>(MultithreadEventLoopGroup.java:52)
	at io.netty.channel.nio.NioEventLoopGroup.<init>(NioEventLoopGroup.java:96)
	at io.netty.channel.nio.NioEventLoopGroup.<init>(NioEventLoopGroup.java:91)
	at io.netty.channel.nio.NioEventLoopGroup.<init>(NioEventLoopGroup.java:72)
	at io.netty.channel.nio.NioEventLoopGroup.<init>(NioEventLoopGroup.java:52)
	at net.pms.network.HTTPServer.start(HTTPServer.java:117)
	at net.pms.PMS.init(PMS.java:630)
	at net.pms.PMS.createInstance(PMS.java:917)
	at net.pms.PMS.main(PMS.java:1087)

and

[main] jdk.internal.misc.Unsafe.allocateUninitializedArray(int): unavailable
java.lang.IllegalAccessException: class io.netty.util.internal.PlatformDependent0$6 cannot access class jdk.internal.misc.Unsafe (in module java.base) because module java.base does not export jdk.internal.misc to unnamed module @409bf450
	at java.base/jdk.internal.reflect.Reflection.newIllegalAccessException(Unknown Source)
	at java.base/java.lang.reflect.AccessibleObject.checkAccess(Unknown Source)
	at java.base/java.lang.reflect.Method.invoke(Unknown Source)
	at io.netty.util.internal.PlatformDependent0$6.run(PlatformDependent0.java:347)
	at java.base/java.security.AccessController.doPrivileged(Unknown Source)
	at io.netty.util.internal.PlatformDependent0.<clinit>(PlatformDependent0.java:338)
	at io.netty.util.internal.PlatformDependent.isAndroid(PlatformDependent.java:289)
	at io.netty.util.internal.PlatformDependent.<clinit>(PlatformDependent.java:92)
	at io.netty.channel.nio.NioEventLoop.newTaskQueue0(NioEventLoop.java:279)
	at io.netty.channel.nio.NioEventLoop.newTaskQueue(NioEventLoop.java:150)
	at io.netty.channel.nio.NioEventLoop.<init>(NioEventLoop.java:138)
	at io.netty.channel.nio.NioEventLoopGroup.newChild(NioEventLoopGroup.java:146)
	at io.netty.channel.nio.NioEventLoopGroup.newChild(NioEventLoopGroup.java:37)
	at io.netty.util.concurrent.MultithreadEventExecutorGroup.<init>(MultithreadEventExecutorGroup.java:84)
	at io.netty.util.concurrent.MultithreadEventExecutorGroup.<init>(MultithreadEventExecutorGroup.java:58)
	at io.netty.channel.MultithreadEventLoopGroup.<init>(MultithreadEventLoopGroup.java:52)
	at io.netty.channel.nio.NioEventLoopGroup.<init>(NioEventLoopGroup.java:96)
	at io.netty.channel.nio.NioEventLoopGroup.<init>(NioEventLoopGroup.java:91)
	at io.netty.channel.nio.NioEventLoopGroup.<init>(NioEventLoopGroup.java:72)
	at io.netty.channel.nio.NioEventLoopGroup.<init>(NioEventLoopGroup.java:52)
	at net.pms.network.HTTPServer.start(HTTPServer.java:117)
	at net.pms.PMS.init(PMS.java:630)
	at net.pms.PMS.createInstance(PMS.java:917)
	at net.pms.PMS.main(PMS.java:1087)

@valib
Copy link
Contributor Author

valib commented Jun 5, 2020

@SubJunk it seems like known problem in Netty started with Java 9. See netty/netty#7769

this is expected and just a debug statement...

@valib
Copy link
Contributor Author

valib commented Jun 5, 2020

@SubJunk other solution for java 11 is here https://stackoverflow.com/questions/57885828/netty-cannot-access-class-jdk-internal-misc-unsafe which works also for Java 14

@SubJunk
Copy link
Member

SubJunk commented Jun 9, 2020

@valib ok I'm happy to leave it since it doesn't seem like something to worry about

@valib
Copy link
Contributor Author

valib commented Jun 10, 2020

@SubJunk agree

SubJunk added a commit that referenced this pull request Jun 15, 2020
SubJunk added a commit that referenced this pull request Jun 15, 2020
* Revert "Fixes ignoring any cling renderers on the same host address (#2090)"

This reverts commit c36a81b.

* Revert "fix checking the internal UMS Cling requests (#2086)"

This reverts commit c71a2f8.

* Revert "response correctly for not implemented request (#2094)"

This reverts commit 5013eda.

* Revert "Implementation of the latest stable Netty4 version (#902)"

This reverts commit 2e1bfa9.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants