Motivation:
ResourceLeakDetector shows two main problems, racy access and heavy lock contention.
Modifications:
This PR fixes this by doing two things:
1. Replace the sampling counter with a ThreadLocalRandom. This has two benefits.
First, it makes the sampling ration no longer have to be a power of two. Second,
it de-noises the continuous races that fight over this single value. Instead,
this change uses slightly more CPU to decide if it should sample by using TLR.
2. DefaultResourceLeaks need to be kept alive in order to catch leaks. The means
by which this happens is by a singular, doubly-linked list. This creates a
large amount of contention when allocating quickly. This is noticeable when
running on a multi core machine.
Instead, this uses a concurrent hash map to keep track of active resources
which has much better contention characteristics.
Results:
Better concurrent hygiene. Running the gRPC QPS benchmark showed RLD taking about
3 CPU seconds for every 1 wall second when runnign with 12 threads.
There are some minor perks to this as well. DefaultResourceLeak accounting is
moved to a central place which probably has better caching behavior.
Motivation:
We need to ensure we not add the Transfer-Encoding header if the HttpMessage is EOF terminated.
Modifications:
Only add the Transfer-Encoding header if an Content-Length header is present.
Result:
Correctly handle HttpMessage that is EOF terminated.
Motivation:
The previously generated manifest causes a parse exception when loaded into an Apache Felix OSGI container.
Modifications:
Fix parameter delimiter and unbalanced quotes in manifest entry. Suffixed with asterisk so the bundle is resolved on other architectures as well even if native libs won't be loaded.
Result:
Bundle will load properly in OSGI containers.
Motivation:
A new version of centos was released we should verify against it when release.
Modifications:
Bump up version.
Result:
Release on latest centos version.
Motivation
It's possible to extend LocalChannel as well as LocalServerChannel but the LocalServerChannel's serve(peer) method is hardcoded to create only instances of LocalChannel.
Modifications
Add a protected factory method that returns by default new LocalChannel(...) but users may override it to customize it.
Result
It's possible to customize the LocalChannel instance on either end of the virtual connection.
Motivation:
In some ByteBuf implementations we not correctly implement getBytes(index, ByteBuffer).
Modifications:
Correct code to do what is defined in the javadocs and adding test.
Result:
Implementation works as described.
Motivation:
Since Java 7, X509TrustManager implementation is wrapped by a JDK class
called AbstractTrustManagerWrapper, which performs an additional
certificate validation for Socket or SSLEngine-backed connections.
This makes the TrustManager implementations provided by
InsecureTrustManagerFactory and FingerprintTrustManagerFactory not
insecure enough, where their certificate validation fails even when it
should pass.
Modifications:
- Add X509TrustManagerWrapper which adapts an X509TrustManager into an
X509ExtendedTrustManager
- Make SimpleTrustManagerFactory wrap an X509TrustManager with
X509TrustManagerWrapper is the provided TrustManager does not extend
X509ExtendedTrustManager
Result:
- InsecureTrustManagerFactory and FingerprintTrustManagerFactory are now
insecure as expected.
- Fixes#5910
Motivation:
8ba5b5f740 removed some ciphers from the default list, and SocketSslEchoTest had one of these ciphers hard coded in the test. The test will fail if the cihper is not supported by default.
Modifications:
SocketSslEchoTest should ensure a cipher is used which will be supported by the peer
Result:
Test result no longer depends upon default cipher list.
Motivation:
Our default cipher list has not been updated in a while. We current support some older ciphers not commonly in use and we don't support some newer ciphers which are more commonly used.
Modifications:
- Update the default list of ciphers for JDK and OpenSSL.
Result:
Default cipher list is more likely to connect to peers.
Fixes https://github.com/netty/netty/issues/5859
Motivation:
Some unit tests in SingleThreadEventLoopTest rely upon Thread.sleep for sequencing events between threads. This can be unreliable and result in spurious test failures if thread scheduling does not occur in a fair predictable manner.
Modifications:
- Reduce the reliance on Thread.sleep in SingleThreadEventLoopTest
Result:
Fixes https://github.com/netty/netty/issues/5851
Motivation:
If the user removes the SslHandler while still in the processing loop we will produce an IllegalReferenceCountException. We should stop looping when the handlerwas removed.
Modifications:
Ensure we stop looping when the handler is removed.
Result:
No more IllegalReferenceCountException.
Motivation:
When using java.nio.DatagramChannel we should not close the channel when a SocketException was thrown as we can still use the channel.
Modifications:
Not close the Channel when SocketException is thrown
Result:
More robust and correct handling of exceptions when using NioDatagramChannel.
Motivation:
If an exception is thrown while processing the ready channels in the EventLoop we should still run all tasks as this may allow to recover. For example a OutOfMemoryError may be thrown and runAllTasks() will free up memory again. Beside this we should also ensure we always allow to shutdown even if an exception was thrown.
Modifications:
- Call runAllTasks() in a finally block
- Ensure shutdown is always handles.
Result:
More robust EventLoop implementations for NIO and Epoll.
Motivation:
We should better first process OP_WRITE before OP_READ as this may allow us to free memory in a faster fashion for previous queued writes.
Modifications:
Process OP_WRITE before OP_READ
Result:
Free memory faster for queued writes.
Motivation:
Add test-case for doing mutal auth with a certificate chain that holds more then one certificate.
Modifications:
Add test case
Result:
more tests.
Motivation:
It'd be usually good to use the latest library version.
Modification:
Bumped JMH to the latest version as of today.
Result:
Now we use JMH version 1.14.1 for our benchmark.
Motivation:
When using the AF_UNIX PROXY protocol, the reader index was not set correctly after consuming the message bytes of the original header ByteBuf. This caused no immediate harm because after the codepath there is no consumer of the ByteBuf in the current implementation. It’s a bug nevertheless, because consumers of the ByteBuf for extensions (like TLVs, which are allowed by the PROXY protocol spec) would consume a ByteBuf that has a wrong readerIndex when using AF_UNIX instead of e.g. IPv4 (which has correct behaviour)
Modifications:
Increase the reader index of the ByteBuf after it was read
Result:
Correct and consistent behaviour of the AF_UNIX codepath
Motivation:
ResourceLeakDetector reports leak for first call to open(obj) as its leakCheckCnt starts with value 0 and increment subsequently. with value of leakCheckCnt =0, it always returns ResourceLeak. Our application calls ResourceLeakDetector.open(obj) to validate Leak and it fails at very first call even though there is no leak in application.
Modifications:
ResourceLeakDetector.leakCheckCnt value will not be 0 while deriving leak and it will not return incorrect value of ResourceLeak.
Result:
Fix false leak report on first call on ResourceLeakDetector.
the implicit #fireChannelReadComplete() in EmbeddedChannel#writeInbound().
Motivation
We use EmbeddedChannels to implement a ProxyChannel of some sorts that shovels
messages between a source and a destination Channel. The latter are real network
channels (such as Epoll) and they may or may not be managed in a ChannelPool. We
could fuse both ends directly together but the EmbeddedChannel provides a nice
disposable section of a ChannelPipeline that can be used to instrument the messages
that are passing through the proxy portion.
The ideal flow looks abount like this:
source#channelRead() -> proxy#writeOutbound() -> destination#write()
source#channelReadComplete() -> proxy#flushOutbound() -> destination#flush()
destination#channelRead() -> proxy#writeInbound() -> source#write()
destination#channelReadComplete() -> proxy#flushInbound() -> source#flush()
The problem is that #writeOutbound() and #writeInbound() emit surplus #flush()
and #fireChannelReadComplete() events which in turn yield to surplus #flush()
calls on both ends of the pipeline.
Modifications
Introduce a new set of write methods that reain the same sematics as the #write()
method and #flushOutbound() and #flushInbound().
Result
It's possible to implement the above ideal flow.
Fix for EmbeddedChannel#ensureOpen() and Unit Tests for it
Some PR stuff.
Motivation:
Currently FlushConsolidationHandler only consolidates if a read loop is
active for a Channel, otherwise each writeAndFlush(...) call will still
be flushed individually. When these calls are close enough, it can be
beneficial to consolidate them even outside of a read loop.
Modifications:
When we allow a flush to "go through", don't perform it immediately, but
submit it on the channel's executor. Under high pressure, this gives
other writes a chance to enqueue before the task gets executed, and so
we flush multiple writes at once.
Result:
Lower CPU usage and less context switching.
Motivation:
We need to ensure we release all direct memory once the DirectPoolArena is collected. Otherwise we may never reclaim the memory and so leak memory.
Modifications:
Ensure we destroy all PoolChunk memory when DirectPoolArena is collected.
Result:
Free up unreleased memory when DirectPoolArena is collected.
Motivation:
HttpObjectDecoder maintains a resetRequested flag which is used to determine if internal state should be reset when a decode occurs. However after a reset is done the resetRequested flag is not set to false. This leads to all data after this point being discarded.
Modifications:
- Set resetRequested to false when a reset is done
Result:
HttpObjectDecoder can still function after a reset.
Motivation:
For the leak profile we attempted to increase the number of leak hints that were retained to make debugging easier, but there was a typo.
Modifications:
- maxRecord -> maxRecords
Result:
Fix typo in pom.xml so leak profile provides more context for leaks.
Motivation
Give the user the ability to back out from SNI negoations.
Modifications
Put a try-catch around the select() call and re-fire any caught Exceptions.
Result
Fixes#5787
Motivation:
Avoid multiple search in fragments map
Modifications:
Replace usage of Map.containsKey by Map.remove
Result:
During packet process, fragment is only search once in the map instead of 3 times in the previous worst case
Motivation:
At the moment only DefaultFileRegion is supported when using the native epoll transport.
Modification:
- Add support for any FileRegion implementation
- Add test case
Result:
Also custom FileRegion implementation are supported when using the epoll transport.
Motivation:
As discussed in #5738, developers need to concern themselves with setting
connection: keep-alive on the response as well as whether to close a
connection or not after writing a response. This leads to special keep-alive
handling logic in many different places. The purpose of the HttpServerKeepAliveHandler
is to allow developers to add this handler to their pipeline and therefore
free themselves of having to worry about the details of how Keep-Alive works.
Modifications:
Added HttpServerKeepAliveHandler to the io.netty.handler.codec.http package.
Result:
Developers can start using HttpServerKeepAliveHandler in their pipeline instead
of worrying about when to close a connection for keep-alive.
Motivation:
IdleStateHandler has a number of volatile member variables which are only accessed from the EventLoop thread. These do not have to be volatile. The accessibility of these member variables are not consistent between private and package private. The state variable can also use a byte instead of an int.
Modifications:
- Remove volatile from member variables
- Change access to private for member variables
- Change state from int to byte
Result:
IdleStateHandler member variables cleaned up.
Motivation:
HttpObjectAggregator has a potential to leak if a new message is received before the existing message has completed, and if a HttpContent is received but maxContentLength has been exceeded, or the content length is too long.
Modifications:
- Make the HttpObjectAggregator more robust to leaks
- Remove the tooLongFrameFound member variable
Result:
More robust HttpObjectAggregator with less chance of leaks
Motivation:
At the moment we log very confusing messages when trying to load a native library which kind of suggest that the whole loading process failed even if just one mechanism failed and the library could be loaded at the end.
Modifications:
Make the mesage less confusing and also log a successful load of the native library.
Result:
Less confusing logs.
Motivation:
IdleStateHandler and ReadTimeoutHandler could mistakely not fire an event even if no channelRead(...) call happened.
Modifications:
Only set lastReadTime if a read happened before.
Result:
More correct IdleStateHandler / ReadTimeoutHandler.
Motivation:
Since netty shaded JCTools the OSGi manifest no longer is correct. It claims to
have an optional import "org.jctools.queues;resolution:=optional,org.jctools.qu
eues.atomic;resolution:=optional,org.jctools.util;resolution:=optional"
However since it is shaded, this is no longer true.
This was noticed when making JCTools a real bundle and netty resolved it as
optional import.
Modifications:
Modify the generated manifest by no longer analyzing org.jctools for imports.
A manual setting of sun.misc as optional was required.
Result:
Netty OSGi bundle will no longer interfere with a JCTools bundle.
Motivation:
To make it easier to debug why notification of a promise failed we should log extra info and make it consistent.
Modifications:
- Create a new PromiseNotificationUtil that has static methods that can be used to try notify a promise and log.
- Reuse this in AbstractChannelHandlerContext, ChannelOutboundBuffer and PromiseNotifier
Result:
Easier to debug why a promise could not be notified.
Motivation:
There is an incoherence in terms of API when one wants to use
startTls: without startTls one can use the SslContextBuilder's
method newHandler, but with startTls, the developper is forced
to call directly the SslHandler constructor.
Modifications:
Introduce startTls as a SslContextBuilder parameter as well as a
member in SslContext (and thus Jdk and OpenSsl implementations!).
Always use this information to call the SslHandler constructor.
Use false by default, in particular in deprecated constructors of
the SSL implementations.
The client Context use false by default
Results:
Fixes#5170 and more generally homogenise the API so that
everything can be done via SslContextBuilder.
Motivation:
The CorsHandler currently closes the channel when it responds to a preflight (OPTIONS)
request or in the event of a short circuit due to failed validation.
Especially in an environment where there's a proxy in front of the service this causes
unnecessary connection churn.
Modifications:
CorsHandler now uses HttpUtil to determine if the connection should be closed
after responding and to set the Connection header on the response.
Result:
Channel will stay open when the CorsHandler responds unless the client specifies otherwise
or the protocol version is HTTP/1.0
Motivation:
af632278d2 introduced a test which only worked on some jvm versions and specific os'es.
Modifications:
Fix test to work on different java versions and os'es
Result:
No flacky test.
Motivation:
We need to ensure we not set duplicated certificates when using OpenSslEngine.
Modifications:
- Skip first cert in chain when set the chain itself and so not send duplicated certificates
- Add interopt unit tests to ensure no duplicates are send.
Result:
No more duplicates.
Motivation:
AbstractTrafficShapingHandler has a package-private method called "userDefinedWritabilityIndex()" which a user may need to override if two sub-classes wants to be used in the ChannelPipeline.
Modifications:
Mark method protected.
Result:
Easier to extend AbstractTrafficShapingHandler.
Motivation:
We can share the code in retain() and retain(...) and also in release() and release(...).
Modifications:
Share code.
Result:
Less duplicated code.
Motivation:
The JDK implementation of SocketChannel has an internal state that is tracked for its operations. Because of this we need to ensure we call finishConnect() before try to call read(...) / write(...) as otherwise it may produce a NotYetConnectedException.
Modifications:
First process OP_CONNECT flag.
Result:
No more possibility of NotYetConnectedException because OP_CONNECT is handled not early enough when processing interestedOps for a Channel.
Motivation:
SslHandler can be cleaned up a bit in terms of naming and duplicated code.
Modifications:
- Fix naming of arguments
- Not schedule timeout event if not really needed
- share some code and simplify
Result:
Cleaner code.
