Motivation:
If AutoClose is false and there is a IoException then AbstractChannel will not close the channel but instead just fail flushed element in the ChannelOutboundBuffer. AbstractChannel also notifies of writability changes, which may lead to an infinite loop if the peer has closed its read side of the socket because we will keep accepting more data but continuously fail because the peer isn't accepting writes.
Modifications:
- If the transport throws on a write we should acknowledge that the output side of the channel has been shutdown and cleanup. If the channel can't accept more data because it is full, and still healthy it is not expected to throw. However if the channel is not healthy it will throw and is not expected to accept any more writes. In this case we should shutdown the output for Channels that support this feature and otherwise just close.
- Connection-less protocols like UDP can remain the same because the channel may disconnected temporarily.
- Make sure AbstractUnsafe#shutdownOutput is called because the shutdown on the socket may throw an exception.
Result:
More correct handling of write failure when AutoClose is false.
Motivation:
ShutdownOutput now fails all pending writes in the ChannelOutboundBuffer and sets it to null. However the Close code path uses the ChannelOutboundBuffer as an indication that the close operation is in progress and exits early and will not call doClose. This will lead to the Channel not actually being fully closed.
Bug introduced by 237a4da1b7
Modifications:
- AbstractChannel#close shouldn't exit early just because outboundBuffer is null, and instead should use additional state closeInitiated to avoid duplicate close operations
Result:
AbstractChannel#close(..) after AbstractChannel#shutdownOutbound() will still invoke doClose and cleanup Channel state.
Motivation:
Netty is unable to use Java9s ALPN support atm.
Modifications:
When running on Java9+ we invoke the correct methods that are exposed on the Java9+ implementation of SSLEngine and so be able to support ALPN.
This patch is based on the work of @rschmitt and so https://github.com/netty/netty/pull/6992.
Result:
Fixes#6933.
Motivation:
netty-tcnative recently change the name of the native libraries from using - to _.
Modifications:
- OpenSsl should use _ instead of - even for the classifiers to be consistent with netty-tcnative
Result:
Loading netty-tcnative works.
Motivation:
We should deprecate ApplicationProtocolNegotiator as the users should use ApplicationProtocolConfig these days.
Modifications:
Add deprecation annotations and javadocs.
Result:
Be able to make package-private in next major release.
Motivation:
We should try to bind to an ipv6 only socket before we enable ipv6 support in the native transport as it may not work due setup of the platform.
Modifications:
Try to bind to ::1 use IPV6 later on if this works
Result:
Fixes [#7021].
Motivation:
Continuing to make netty happy when compiling through errorprone.
Modification:
Mostly comments, some minor switch statement changes.
Result:
No more compiler errors!
Motivation:
When SslHandlerTest#testCompositeBufSizeEstimationGuaranteesSynchronousWrite fails it would be useful to know the SslProvider type
Modifications:
- Print the sever and client SslProvider upon failure
- Increase test timeout to 8 minutes to allow more time to run
Result:
Failures include more info to help diagnose issues.
Motivation:
DelegatingSslContext at the moment intercept newEngine calls and allow to init the SslEngine after it is created. The problem here is that this may not work the SSLEngine that is wrapped in the SslHandler when calling newHandler(...). This is because some SslContext implementations not delegate to newEngine(...) when creating the SslHandler to allow some optimizations. For this we should also allow to init the SslHandler after its creation and by default just delegate to initEngine(...).
Modifications:
Allow the user to also init the SslHandler after creation while by default init its SSLEngine after creation.
Result:
More flexible and correct code.
Motivation:
Remove Unsafe dependency for Atomic queues in JCTools, resolved in version 2.1.0
Modification:
Change pom JCTools version
Result:
Fixes#7117
Motivation:
KQueueEventLoop and EpollEventLoop implement different approaches to applying a timeout of their respective poll calls. Epoll attempts to ensure the desired timeout is satisfied at the java layer and at the JNI layer, but it should be sufficient to account for spurious wakups at the JNI layer. Epoll timeout granularity is also limited to milliseconds which may be too large for some latency sensitive applications.
Modifications:
- Make EpollEventLoop wait method look like KQueueEventLoop
- Epoll should support a finer timeout granularity via timerfd_create. We can hide most of these details behind the epollWait0 JNI call to avoid crossing additional JNI boundaries.
Result:
More consistent timeout approach between KQueue and Epoll.
Motivation:
Calling `newInstance()` on a Class object can bypass compile time
checked Exception propagation. This is noted in Java Puzzlers,
as well as in ErrorProne:
http://errorprone.info/bugpattern/ClassNewInstance
Modifications:
Use the niladic constructor to create a new instance.
Result:
Compile time safety for checked exceptions
This reverts commit d63bb4811e as this not covered correctly all cases and so could lead to missing fireChannelReadComplete() calls. We will re-evalute d63bb4811e and resbumit a pr once we are sure all is handled correctly
Motivation:
We recently changed netty-tcnative to use underscores in its native library names.
Modifications:
Update code to use underscores when loading native library.
Result:
More consistent code.
Motivation:
On restricted systems (e.g. grsecurity), it might not be possible to write a .so on disk and load it afterwards. On those system Netty should check java.library.path for libraries to load.
Modifications:
Changed NativeLibraryLoader.java to first try to load libs from java.library.path before exporting the .so to disk.
Result:
Libraries load fine on restricted systems.
Motivation:
SslHandlerTest#testCompositeBufSizeEstimationGuaranteesSynchronousWrite has been observed to fail on CI servers, but it is not clear why.
Modifications:
- Add more visibility into what the state was and what the condition that caused the failure was.
Result:
More visibility when the test fails.
Motivation:
Commit 4448b8f42f introduced some API breakage which we need to revert before we release.
Modifications:
- Introduce an AllocatorAwareSslEngineWrapperFactory which expose an extra method that takes a ByteBufAllocator as well.
- Revert API changes to SslEngineWrapperFactory.
Result:
API breakage reverted.
Motivation:
In ReadOnlyByteBufferBuf.copy(...) we just allocated a ByteBuffer directly and wrapped it. This way it was not possible for us to free the direct memory that was used by the copy without the GC.
Modifications:
- Ensure we use the allocator when create the copy and so be able to release direct memory in a timely manner
- Add unit test
- Depending on if the to be copied buffer is direct or heap based we also allocate the same type on copy.
Result:
Fixes [#7103].
Motivation:
To be able to build with latest java9 release we need to adjust maven-enforcer-plugin version.
Modifications:
- Use maven-enforcer-plugin 3.0.0.M1 when building with java9
Result:
Netty builds again with latest java9 release
Motivation:
At the moment we try to load the library using multiple names which includes names using - but also _ . We should just use _ all the time.
Modifications:
Replace - with _
Result:
Fixes [#7069]
Motivation:
In some cases of using an `InternalThreadLocalMap#stringBuilder`, the `StringBuilder`s size can often exceed the exist limit (1024 bytes). This can lead to permanent memory reallocation.
Modifications:
Add custom properties for the initial capacity and maximum size (after which the `StringBuilder`s capacity will be reduced to the initial capacity).
Result:
An `InternalThreadLocalMap#stringBuilder`s initial and max size is configurable. Fixes [#7092].
Motivation:
SocketStringEchoTest has been observed to fail on CI servers, but the stack traces still indicate work was being done.
Modifications:
- Increase the test timeout
Result:
Tests have more time to complete, and hopefully less false positive test failures.
Motivation:
Missing return in ByteBufUtil#writeAscii causes endless loop
Modifications:
Add return after write finished
Result:
ByteBufUtil#writeAscii is ok
Motivation:
EpollDomainSocketGatheringWriteTest. testGatheringWriteBig takes on average about 20-25 seconds on the CI servers, but there is a 30 second timeout being applied which leads to what maybe false positive test failures.
Modifications:
- Increase the test timeout to 120 seconds globally and 60 seconds to wait for all writes per test
Result:
Higher timeout for potentially less false positive test failures.
Motivation:
SocketGatherWriteTest has been observed to fail and it has numerous issues which when resolved may help reduce the test failures.
Modifications:
- A volatile counter and a spin/sleep loop is used to trigger test termination. Incrementing a volatile is generally bad practice and can be avoided in this situation. This mechanism can be replaced by a promise. This mechanism should also trigger upon exception or channel inactive.
- The TestHandler maintains an internal buffer, but it is not released. We now only create a buffer on the server side and release it after comparing the expected results.
- The composite buffer creation logic can be simplified, also the existing composite buffer doesn't take into account the buffer's reader index when building buf2.
Result:
Cleaner test.
Motivation:
SocketStringEchoTest has been observed to fail and it has numerous issues which when resolved may help reduce the test failures.
Modifications:
- A volatile counter and a spin/sleep loop is used to trigger test termination. Incrementing a volatile is generally bad practice and can be avoided in this situation. This mechanism can be replaced by a promise. This mechanism should also trigger upon exception or channel inactive.
- Asserts are done in the Netty threads. Although these should result in a exceptionCaught the test may not observe these failures because it is spinning waiting for the count to reach the desired value.
Result:
Cleaner test.
Motivation:
We need to ensure we always null out (or set) the address on the java.net.DatagramPacket when doing read or write operation as the same instance is used across different calls.
Modifications:
Null out the address if needed.
Result:
Ensure the correct remote address is used when connect / disconnect between calls and also mix these with calls that directly specify the remote address for adatagram packets.
Motivation:
We need to support SO_TIMEOUT for the OioDatagramChannel but we miss this atm as we not have special handling for it in the DatagramChannelConfig impl that we use. Because of this the following log lines showed up when running the testsuite:
20:31:26.299 [main] WARN io.netty.bootstrap.Bootstrap - Unknown channel option 'SO_TIMEOUT' for channel '[id: 0x7cb9183c]'
Modifications:
- Add OioDatagramChannelConfig and impl
- Correctly set SO_TIMEOUT in testsuite
Result:
Support SO_TIMEOUT for OioDatagramChannel and so faster execution of datagram related tests in the testsuite
Motivation:
NativeLibraryLoader has some code-duplication that can be removed.
Modifications:
Remove duplicated code and just use provided methods of PlatformDependent.
Result:
Less code duplication, fixes [#3756].
Motivation:
Issue #6695 states that there is an issue when writing empty content via HttpResponseEncoder.
Modifications:
Add two test-cases.
Result:
Verified that all works as expected.
Motivation:
We used asm 6.0_ALPHA when building on java9 as the latest stable release not works with java9. asm 6.0_BETA was just released so we should update.
Modifications:
Upgrade asm version
Result:
Not use ALPHA release anymore
Motivation:
Implementations of DuplexChannel delegate the shutdownOutput to the underlying transport, but do not take any action on the ChannelOutboundBuffer. In the event of a write failure due to the underlying transport failing and application may attempt to shutdown the output and allow the read side the transport to finish and detect the close. However this may result in an issue where writes are failed, this generates a writability change, we continue to write more data, and this may lead to another writability change, and this loop may continue. Shutting down the output should fail all pending writes and not allow any future writes to avoid this scenario.
Modifications:
- Implementations of DuplexChannel should null out the ChannelOutboundBuffer and fail all pending writes
Result:
More controlled sequencing for shutting down the output side of a channel.
Motivation:
Its wasteful and also confusing that channelReadComplete() is called even if there was no message forwarded to the next handler.
Modifications:
- Only call ctx.fireChannelReadComplete() if at least one message was decoded
- Add unit test
Result:
Less confusing behavior. Fixes [#4312].
Motivation:
Now that the NativeLibraryLoader implicitly detects the shaded package prefix we no longer need the io.netty.packagePrefix system property.
Modifications:
- Remove io.netty.packagePrefix processing from NativeLibraryLoader
Result:
Code is cleaner.
Motivation:
We need to ensure we not try to load any conscrypt classes directly (which means without using reflection) in the same class that is used to check if conscrypt is available. This is needed as otherwise we will have the following problem when try to use netty on java7:
java.lang.UnsupportedClassVersionError: org/conscrypt/BufferAllocator : Unsupported major.minor version 52.0
at io.netty.handler.ssl.ConscryptJdkSslEngineInteropTest.checkConscrypt(ConscryptJdkSslEngineInteropTest.java:49)
This regression was introduced by 4448b8f42f and detected on the CI when using:
mvn clean package -DtestJavaHome=$JAVA7_HOME
Modifications:
Move the detection code in an extra class and use it.
Result:
Works correctly also when using Java7.
Motivation:
We did not correctly handle connect() and disconnect() in EpollDatagramChannel and so the behavior was different compared to NioDatagramChannel.
Modifications:
- Correct implement connect and disconnect methods
- Share connect and related code
- Add tests
Result:
EpollDatagramChannel also supports correctly connect() and disconnect() methods.
Motivation:
Starting with 1.0.0.RC9, conscrypt supports a buffer allocator.
Modifications:
- Updated the creation process for the engine to pass through the
ByteBufAllocator.
- Wrap a ByteBufAllocator with an adapter for conscrypt.
- Added a property to optionally control whether conscrypt uses
Netty's buffer allocator.
Result:
Netty+conscrypt will support using Netty's ByteBufAllocator.
Motivation:
1. Hash function in the Snappy encoding is wrong probably: used '+' instead of '*'. See the reference implementation [1].
2. Size of the hash table is calculated, but not applied.
Modifications:
1. Fix hash function: replace addition by multiplication.
2. Allocate hash table with calculated size.
3. Use an `Integer.numberOfLeadingZeros` trick for calculate log2.
4. Release buffers in tests.
Result:
1. Better compression. In the test `encodeAndDecodeLongTextUsesCopy` now compressed size is 175 instead of 180 before this change.
2. No redundant allocations for hash table.
3. A bit faster the calc of shift (less an expensive math operations).
[1] 513df5fb5a/snappy.cc (L67)
Motivation:
IP_TRANSPARENT support is not complete, the option can currently only be set on EpollServerSocket. Setting the option on an EpollSocket is also requires so as to be able to bind a socket to a non-local address as described in ip(7)
http://man7.org/linux/man-pages/man7/ip.7.html
"TProxy redirection with the iptables TPROXY target also
requires that this option be set on the redirected socket."
Modifications:
Added IP_TRANSPARENT socket option to EpollSocketChannelConfig
Result:
A redirecting socket can be created with a non-local IP address as required for TPROXY
Motivation:
When a user called ctx.close() and used the EmbeddedChannel we did not correctly run all pending tasks which means channelInactive was never called.
Modifications:
Ensure we run all pending tasks after all operations that may change the Channel state and are part of the Channel.Unsafe impl.
Result:
Fixes [#6894].
