Netty

Clone Tools
  • last updated a few seconds ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
Allow ndots=0 in DnsNameResolver and search domains - fixes #5570

Motivation:

The ndots = 0 is a valid value for ndots, it means that when using a non dotted name, the resolution should first try using a search and if it fails then use subdomains. Currently it is not allowed. Docker compose uses this when wiring up containers as names have usually no dots inside.

Modification:

Modify DnsNameResolver to accept ndots = 0 and handle the case in the resolution procedure. In this case a direct search is done and then a fallback on the search path is performed.

Result:

The ndots = 0 case is implemented.

[#5551] Fix sporadic GlobalEventExecutorTest.testAutomaticStartStop test failure

Motivation:

We saw some sporadic test failures for GlobalEventExecutorTest.testAutomaticStartStop test. This is caused parallel execution of tests in combination with assert checks that will be affected.

Modifications:

Remove fragile assert checks.

Result:

No more sporadic test failures

[#5551] Fix sporadic GlobalEventExecutorTest.testAutomaticStartStop test failure

Motivation:

We saw some sporadic test failures for GlobalEventExecutorTest.testAutomaticStartStop test. This is caused parallel execution of tests in combination with assert checks that will be affected.

Modifications:

Remove fragile assert checks.

Result:

No more sporadic test failures

Ensure WeakOrderQueue can be collected fast enough

Motivation:

Commit afafadd3d7caf1e4b346da049baab0afeae0a4bc introduced a change which stored the Stack in the WeakOrderQueue as field. This unfortunally had the effect that it was not removed from the WeakHashMap anymore as the Stack also is used as key.

Modifications:

Do not store a reference to the Stack in WeakOrderQueue.

Result:

WeakOrderQueue can be collected correctly again.

    • -24
    • +29
    /common/src/main/java/io/netty/util/Recycler.java
Ensure WeakOrderQueue can be collected fast enough

Motivation:

Commit afafadd3d7caf1e4b346da049baab0afeae0a4bc introduced a change which stored the Stack in the WeakOrderQueue as field. This unfortunally had the effect that it was not removed from the WeakHashMap anymore as the Stack also is used as key.

Modifications:

Do not store a reference to the Stack in WeakOrderQueue.

Result:

WeakOrderQueue can be collected correctly again.

    • -24
    • +29
    /common/src/main/java/io/netty/util/Recycler.java
Ensure shared capacity is updated correctly when WeakOrderQueue is collected.

Motivation:

We use a shared capacity per Stack for all its WeakOrderQueue elements. These relations are stored in a WeakHashMap to allow dropping these if memory pressure arise. The problem is that we not "reclaim" previous reserved space when this happens. This can lead to a Stack which has not shared capacity left which then will lead to an AssertError when we try to allocate a new WeakOderQueue.

Modifications:

- Ensure we never throw an AssertError if we not have enough space left for a new WeakOrderQueue

- Ensure we reclaim space when WeakOrderQueue is collected.

Result:

No more AssertError possible when new WeakOrderQueue is created and also correctly reclaim space that was reserved from the shared capacity.

    • -7
    • +32
    /common/src/main/java/io/netty/util/Recycler.java
Ensure shared capacity is updated correctly when WeakOrderQueue is collected.

Motivation:

We use a shared capacity per Stack for all its WeakOrderQueue elements. These relations are stored in a WeakHashMap to allow dropping these if memory pressure arise. The problem is that we not "reclaim" previous reserved space when this happens. This can lead to a Stack which has not shared capacity left which then will lead to an AssertError when we try to allocate a new WeakOderQueue.

Modifications:

- Ensure we never throw an AssertError if we not have enough space left for a new WeakOrderQueue

- Ensure we reclaim space when WeakOrderQueue is collected.

Result:

No more AssertError possible when new WeakOrderQueue is created and also correctly reclaim space that was reserved from the shared capacity.

    • -7
    • +32
    /common/src/main/java/io/netty/util/Recycler.java
[#5566] Ensure using a ChannelInitializer via ServerBootstrap.handler(...) produce correct ordering.

Motivation:

When a ChannelInitializer is used via ServerBootstrap.handler(...) the users handlers may be added after the internal ServerBootstrapAcceptor. This should not happen.

Modifications:

Delay the adding of the ServerBootstrapAcceptor until the initChannel(....) method returns.

Result:

Correct order of handlers in the ServerChannels ChannelPipeline.

[#5566] Ensure using a ChannelInitializer via ServerBootstrap.handler(...) produce correct ordering.

Motivation:

When a ChannelInitializer is used via ServerBootstrap.handler(...) the users handlers may be added after the internal ServerBootstrapAcceptor. This should not happen.

Modifications:

Delay the adding of the ServerBootstrapAcceptor until the initChannel(....) method returns.

Result:

Correct order of handlers in the ServerChannels ChannelPipeline.

Elliminated some buggy behavior when using a KeyManagerFactory with OpenSslServerContext.

Motivation:

PR #5493 added support for KeyManagerFactories when using the OpenSsl context. This commit corrects a bug causing a NullPointerException that occurs when using a KeyManagerFactory without a certificate chain and private key.

Modifications:

Removes assertNotNull() assertions which were causing a certificate chain and private key to be required even when using a KeyManagerFactory. Also removed a redundant call to buildKeyManagerFactory() which was also causing a exception when a KeyManagerFactory is provided but a certificate chain and private key is not.

Result:

A KeyManagerFactory can now be used in the OpenSslServerContext without an independent certificate chain and private key.

Elliminated some buggy behavior when using a KeyManagerFactory with OpenSslServerContext.

Motivation:

PR #5493 added support for KeyManagerFactories when using the OpenSsl context. This commit corrects a bug causing a NullPointerException that occurs when using a KeyManagerFactory without a certificate chain and private key.

Modifications:

Removes assertNotNull() assertions which were causing a certificate chain and private key to be required even when using a KeyManagerFactory. Also removed a redundant call to buildKeyManagerFactory() which was also causing a exception when a KeyManagerFactory is provided but a certificate chain and private key is not.

Result:

A KeyManagerFactory can now be used in the OpenSslServerContext without an independent certificate chain and private key.

Ensure attributes and contained object can be collected as fast as possible.

Motivation:

Due an implementation flaw in DefaultAttributeMap it was possible that an attribute and its stored value/key could not be collected until the DefaultAttributeMap could be collected. This can lead to unexpected memory usage and strong reachability of objects that should be collected.

Modifications:

Use an special empty DefaultAttribute as head of the each bucket which will not store any key / value. With this change everything can be collected as expected as we not use any DefaultAttribute created by the user as head of a bucket.

Result:

DefaultAttributeMap does not store user data and thus the lifetime of this user data is not tied to the lifetime of the DefaultAttributeMap.

Ensure attributes and contained object can be collected as fast as possible.

Motivation:

Due an implementation flaw in DefaultAttributeMap it was possible that an attribute and its stored value/key could not be collected until the DefaultAttributeMap could be collected. This can lead to unexpected memory usage and strong reachability of objects that should be collected.

Modifications:

Use an special empty DefaultAttribute as head of the each bucket which will not store any key / value. With this change everything can be collected as expected as we not use any DefaultAttribute created by the user as head of a bucket.

Result:

DefaultAttributeMap does not store user data and thus the lifetime of this user data is not tied to the lifetime of the DefaultAttributeMap.

Reduce conditionals in AbstractReferenceCounted

Motivation:

AbstractReferenceCounted as independent conditional statements to check the bounds of the retain IllegalReferenceCountException condition. One of the exceptions also uses the incorrect increment.

Modifications:

- Combined independent conditional checks into 1 where possible

- Correct IllegalReferenceCountException with incorrect increment

- Remove the subtract to check for overflow and re-use the addition and check for overflow to remove 1 arithmetic operation (see http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.18.2)

Result:

AbstractReferenceCounted has less independent branch statements and more correct IllegalReferenceCountException. Compilation size of AbstractReferenceCounted.retain() is reduced from 58 bytes to 47 bytes.

Reduce conditionals in AbstractReferenceCounted

Motivation:

AbstractReferenceCounted as independent conditional statements to check the bounds of the retain IllegalReferenceCountException condition. One of the exceptions also uses the incorrect increment.

Modifications:

- Combined independent conditional checks into 1 where possible

- Correct IllegalReferenceCountException with incorrect increment

- Remove the subtract to check for overflow and re-use the addition and check for overflow to remove 1 arithmetic operation (see http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.18.2)

Result:

AbstractReferenceCounted has less independent branch statements and more correct IllegalReferenceCountException. Compilation size of AbstractReferenceCounted.retain() is reduced from 58 bytes to 47 bytes.

Reduce permissions needed for process ID

Motiviation:

DefaultChannelId attempts to acquire a default process ID by determining

the process PID. However, to do this it attempts to punch through to the

system classloader, a permission that in the face of a restrictive

security manager is unlikely to be granted. Looking past this, it then

attempts to load a declared method off a reflectively loaded class,

another permission that is not likely to be granted in the face of a

restrictive security manager. However, neither of these permissions are

necessary as the punching through to the system security manager is

completely unneeded, and there is no need to load a public method as a

declared method.

Modifications:

Instead of punching through to the system classloader requiring

restricted permissions, we can just use current classloader. To address

the access declared method permission, we instead just reflectively

obtain the desired public method via Class#getMethod.

Result:

Acquiring the default process ID from the PID will succeed without

requiring the runtime permissions "getClassLoader" and

"accessDeclaredMembers".

No need to do a memory copy to verify snappy identifier

Motivation:

We are currently doing a memory copy to verify the snapy version. This is not needed.

Modifications:

Remove memory copy and just compare byte per byte.

Result:

Less memory copies and allocations

Ensure uncompressed ByteBuf is released when an exception happens during decoding.

Motivation:

We need to ensure the uncompressed ByteBuf is released if an exception happens while calling decode(...). If we miss to do so we leak buffers.

Modifications:

Correctly release buffer on exception.

Result:

No more memory leak.

Construct LOCALHOST4 and LOCALHOST6 object with hostname "localhost"

Motivation:

When resolving localhost on Windows where the hosts file does not contain a localhost entry by default, the resulting InetAddress object returned by the resolver does not have the hostname set so that getHostName returns the ip address 127.0.0.1. This behaviour is inconsistent with Windows where the hosts file does contain a localhost entry and with Linux in any case. It breaks at least some unit tests.

Modifications:

Create the LOCALHOST4 and LOCALHOST6 objects with hostname localhost in addition to the address.

Add unit test domain localhost to DnsNameResolverTest to check the resolution of localhost with ipv4 at least.

Result:

The resolver returns a InetAddress object for localhost with the hostname localhost in all cases.

[#5553] SimpleChannelPool#notifyConnect() may leak Channels

Motivation:

The SimpleChannelPool#notifyConnect() method will leak Channels if the user cancelled the Promise in between.

Modifications:

Release the channel if the Promise was complete before.

Result:

No more channel leaks.

[#5553] SimpleChannelPool#notifyConnect() may leak Channels

Motivation:

The SimpleChannelPool#notifyConnect() method will leak Channels if the user cancelled the Promise in between.

Modifications:

Release the channel if the Promise was complete before.

Result:

No more channel leaks.

Add NonStickyEventExecutorGroup

Motivation:

We offer DefaultEventExecutorGroup as an EventExecutorGroup which return OrderedEventExecutor and so provide strict ordering of event execution. One limitations of this implementation is that each contained DefaultEventExecutor will always be tied to a single thread, which can lead to a very unbalanced execution as one thread may be super busy while others are idling.

Modifications:

- Add NonStickyEventExecutorGroup which can be used to wrap another EventExecutorGroup (like UnorderedThreadPoolEventExecutor) and expose ordering while not be sticky with the thread that is used for a given EventExecutor. This basically means that Threads may change between execution of tasks for an EventExecutor but ordering is still guaranteed.

Result:

Better utalization of threads in some use-cases.

Remove unnessary memory copy when doing Bzip2 encoding

Motivation:

We did an unessary memory copy when doing bzip2 encoding.

Modifications:

Remove memory copy and just use a ByteProcessor.

Result:

Less memory copies and so faster.

Remove memory copies when doing CRC32 processing in JdkZlibDecoder

Motivation:

We not need to do any memory copies when doing CRC32 processing.

Modifications:

Use ByteBufChecksum to eliminate memory copies.

Result:

Less memory copies.

Remove memory copy when checksum non heap backed ByteBuf implementations using Snappy

Motivation:

We should try to minimize memory copies whenever possible.

Modifications:

- Refactor ByteBufChecksum to work with heap and direct ByteBuf always

- Remove memory copy in Snappy by let Crc32c extend ByteBufChecksum

Result:

Less memory copies when using Snappy

Acquire Java version simply

Motivation:

The Java version is used for platform dependent logic. Yet, the logic

for acquiring the Java version requires special permissions (the runtime

permission "getClassLoader") that some downstream projects will never

grant. As such, these projects are doomed to have Netty act is their

Java major version is six. While there are ways to maintain the same

logic without requiring these special permissions, the logic is

needlessly complicated because it relies on loading classes that exist

in version n but not version n - 1. This complexity can be removed. As a

bonanza, the dangerous permission is no longer required.

Modifications:

Rather than attempting to load classes that exist in version n but not

in version n - 1, we can just parse the Java specification version. This

only requires a begign property (property permission

"java.specification.version") and is simple.

Result:

Acquisition of the Java version is safe and simple.

Acquire Java version simply

Motivation:

The Java version is used for platform dependent logic. Yet, the logic

for acquiring the Java version requires special permissions (the runtime

permission "getClassLoader") that some downstream projects will never

grant. As such, these projects are doomed to have Netty act is their

Java major version is six. While there are ways to maintain the same

logic without requiring these special permissions, the logic is

needlessly complicated because it relies on loading classes that exist

in version n but not version n - 1. This complexity can be removed. As a

bonanza, the dangerous permission is no longer required.

Modifications:

Rather than attempting to load classes that exist in version n but not

in version n - 1, we can just parse the Java specification version. This

only requires a begign property (property permission

"java.specification.version") and is simple.

Result:

Acquisition of the Java version is safe and simple.

Make Epoll ChannelMetadata more consistent with NIO

Motivation:

In 4.0 AbstractNioByteChannel has a default of 16 max messages per read. However in 4.1 that constraint was applied at the NioSocketChannel which is not equivalent. In 4.1 AbstractEpollStreamChannel also did not have the default of 16 max messages per read applied.

Modifications:

- Make Nio consistent with 4.0

- Make Epoll consistent with Nio

Result:

Nio and Epoll both have consistent ChannelMetadata and are consistent with 4.0.

Fix the tcnative lib loading problem in OSGi

Motivation:

As the issue #5539 say, the OpenSsl.class will throw `java.lang.UnsatisfiedLinkError: org.apache.tomcat.jni.Library.version(I)I` when it is invoked. This path try to resolve the problem by modifying the native library loading logic of OpenSsl.class.

Modifications:

The OpenSsl.class loads the tcnative lib by `NativeLibraryLoader.loadFirstAvailable()`. The native library will be loaded in the bundle `netty-common`'s ClassLoader, which is diff with the native class's ClassLoader. That is the root cause of throws `UnsatisfiedLinkError` when the native method is invoked.

So, it should load the native library by the its bundle classloader firstly, then the embedded resources if failed.

Result:

First of all, the error threw by native method problem will be resolved.

Secondly, the native library should work as normal in non-OSGi env. But, this is hard. The loading logic of `Library.class` in `netty-tcnative` bundle is simple: try to load the library in PATH env. If not found, it falls back to the originally logic `NativeLibraryLoader.loadFirstAvailable()`.

Signed-off-by: XU JINCHUAN <xsir@msn.com>

Fix the tcnative lib loading problem in OSGi

Motivation:

As the issue #5539 say, the OpenSsl.class will throw `java.lang.UnsatisfiedLinkError: org.apache.tomcat.jni.Library.version(I)I` when it is invoked. This path try to resolve the problem by modifying the native library loading logic of OpenSsl.class.

Modifications:

The OpenSsl.class loads the tcnative lib by `NativeLibraryLoader.loadFirstAvailable()`. The native library will be loaded in the bundle `netty-common`'s ClassLoader, which is diff with the native class's ClassLoader. That is the root cause of throws `UnsatisfiedLinkError` when the native method is invoked.

So, it should load the native library by the its bundle classloader firstly, then the embedded resources if failed.

Result:

First of all, the error threw by native method problem will be resolved.

Secondly, the native library should work as normal in non-OSGi env. But, this is hard. The loading logic of `Library.class` in `netty-tcnative` bundle is simple: try to load the library in PATH env. If not found, it falls back to the originally logic `NativeLibraryLoader.loadFirstAvailable()`.

Signed-off-by: XU JINCHUAN <xsir@msn.com>