Motivation:
Commit eb0e127ee9 was designed for the 4.1 branch. The 5.0 branch had a few additional classes which required updates to get the new timeMillis methods for all the header classes.
Modifications:
Update the *Headers interfaces/classes that were not updated in the 4.1 branch.
Result:
All *Headers interfaces support set/add timeMillis methods.
Motivation:
We expose no methods in ByteBuf to directly write a CharSequence into it. This leads to have the user either convert the CharSequence first to a byte array or use CharsetEncoder. Both cases have some overheads and we can do a lot better for well known Charsets like UTF-8 and ASCII.
Modifications:
Add ByteBufUtil.writeAscii(...) and ByteBufUtil.writeUtf8(...) which can do the task in an optimized way. This is especially true if the passed in ByteBuf extends AbstractByteBuf which is true for all of our implementations which not wrap another ByteBuf.
Result:
Writing an ASCII and UTF-8 CharSequence into a AbstractByteBuf is a lot faster then what the user could do by himself as we can make use of some package private methods and so eliminate reference and range checks. When the Charseq is not ASCII or UTF-8 we can still do a very good job and are on par in most of the cases with what the user would do.
The following benchmark shows the improvements:
Result: 2456866.966 ?(99.9%) 59066.370 ops/s [Average]
Statistics: (min, avg, max) = (2297025.189, 2456866.966, 2586003.225), stdev = 78851.914
Confidence interval (99.9%): [2397800.596, 2515933.336]
Benchmark Mode Samples Score Score error Units
i.n.m.b.ByteBufUtilBenchmark.writeAscii thrpt 50 9398165.238 131503.098 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeAsciiString thrpt 50 9695177.968 176684.821 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeAsciiStringViaArray thrpt 50 4788597.415 83181.549 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeAsciiStringViaArrayWrapped thrpt 50 4722297.435 98984.491 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeAsciiStringWrapped thrpt 50 4028689.762 66192.505 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeAsciiViaArray thrpt 50 3234841.565 91308.009 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeAsciiViaArrayWrapped thrpt 50 3311387.474 39018.933 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeAsciiWrapped thrpt 50 3379764.250 66735.415 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeUtf8 thrpt 50 5671116.821 101760.081 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeUtf8String thrpt 50 5682733.440 111874.084 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeUtf8StringViaArray thrpt 50 3564548.995 55709.512 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeUtf8StringViaArrayWrapped thrpt 50 3621053.671 47632.820 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeUtf8StringWrapped thrpt 50 2634029.071 52304.876 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeUtf8ViaArray thrpt 50 3397049.332 57784.119 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeUtf8ViaArrayWrapped thrpt 50 3318685.262 35869.562 ops/s
i.n.m.b.ByteBufUtilBenchmark.writeUtf8Wrapped thrpt 50 2473791.249 46423.114 ops/s
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1,387.417 sec - in io.netty.microbench.buffer.ByteBufUtilBenchmark
Results :
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
Results :
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
The *ViaArray* benchmarks are basically doing a toString().getBytes(Charset) which the others are using ByteBufUtil.write*(...).
Motivation:
ProxyHandlerTest fails with NoClassDefFoundError raised by
SslContext.newClientContext().
Modifications:
Fix a missing 'return' statement that makes the switch-case block fall
through unncecessarily
Result:
- ProxyHandlerTest does not fail anymore.
- SslContext.newClientContext() does not raise NoClassDefFoundError
anymore.
Motivation:
HEAD requests will have a Content-Length set that doesn't match the
actual length. So we only want to set Content-Length header if it isn't
already set.
Modifications:
If check around setting the Content-Length.
Result:
A HEAD request will no correctly return the specified Content-Length
instead of the body length.
Motivation:
At the moment we use SSL.getLastError() in unwrap(...) to check for error. This is very inefficient as it creates a new String for each check and we also use a String.startsWith(...) to detect if there was an error we need to handle.
Modifications:
Use SSL.getLastErrorNumber() to detect if we need to handle an error, as this only returns a long and so no String creation happens. Also the detection is much cheaper as we can now only compare longs. Once an error is detected the lately SSL.getErrorString(long) is used to conver the error number to a String and include it in log and exception message.
Result:
Performance improvements in OpenSslEngine.unwrap(...) due less object allocation and also faster comparations.
Motivation:
As we now support OpenSslEngine for client side, we should use it when avaible.
Modifications:
Use SslProvider.OPENSSL when openssl can be found
Result:
OpenSslEngine is used whenever possible
Motivation:
When using client auth it is sometimes needed to use a custom TrustManagerFactory.
Modifications:
Allow to pass in TrustManagerFactory
Result:
It's now possible to use custom TrustManagerFactories for JdkSslServerContext and OpenSslServerContext
Motivation:
To make OpenSsl*Context a drop in replacement for JdkSsl*Context we need to use TrustManager.
Modifications:
Correctly hook in the TrustManager
Result:
Better compatibility
Motivation:
At the moment there is no way to enable client authentication when using OpenSslEngine. This limits the uses of OpenSslEngine.
Modifications:
Add support for different authentication modes.
Result:
OpenSslEngine can now also be used when client authenticiation is needed.
Motivation:
The current SSLSession implementation used by OpenSslEngine does not support various operations and so may not be a good replacement by the SSLEngine provided by the JDK implementation.
Modifications:
- Add SSLSession.getCreationTime()
- Add SSLSession.getLastAccessedTime()
- Add SSLSession.putValue(...), getValue(...), removeValue(...), getValueNames()
- Add correct SSLSession.getProtocol()
- Ensure OpenSSLEngine.getSession() is thread-safe
- Use optimized AtomicIntegerFieldUpdater when possible
Result:
More complete OpenSslEngine SSLSession implementation
Motivation:
We only support openssl for server side at the moment but it would be also useful for client side.
Modification:
* Upgrade to new netty-tcnative snapshot to support client side openssl support
* Add OpenSslClientContext which can be used to create SslEngine for client side usage
* Factor out common logic between OpenSslClientContext and OpenSslServerContent into new abstract base class called OpenSslContext
* Correctly detect handshake failures as soon as possible
* Guard against segfault caused by multiple calls to destroyPools(). This can happen if OpenSslContext throws an exception in the constructor and the finalize() method is called later during GC
Result:
openssl can be used for client and servers now.
Motivation:
SslHandler.wrap(...) does a poor job when handling CompositeByteBuf as it always call ByteBuf.nioBuffer() which will do a memory copy when a CompositeByteBuf is used that is backed by multiple ByteBuf.
Modifications:
- Use SslEngine.wrap(ByteBuffer[]...) to allow wrap CompositeByteBuf in an efficient manner
- Reduce object allocation in unwrapNonAppData(...)
Result:
Performance improvement when a CompositeByteBuf is written and the SslHandler is in the ChannelPipeline.
Motivation:
CompositeByteBuf.nioBuffers(...) returns an empty ByteBuffer array if the specified length is 0. This is not consistent with other ByteBuf implementations which return an ByteBuffer array of size 1 with an empty ByteBuffer included.
Modifications:
Make CompositeByteBuf.nioBuffers(...) consistent with other ByteBuf implementations.
Result:
Consistent and correct behaviour of nioBufffers(...)
Motivation:
When calling slice(...) on a ByteBuf the returned ByteBuf should be the slice of a ByteBuf and shares it's reference count. This is important as it is perfect legal to use buf.slice(...).release() and have both, the slice and the original ByteBuf released. At the moment this is only the case if the requested slice size is > 0. This makes the behavior inconsistent and so may lead to a memory leak.
Modifications:
- Never return Unpooled.EMPTY_BUFFER when calling slice(...).
- Adding test case for buffer.slice(...).release() and buffer.duplicate(...).release()
Result:
Consistent behaviour and so no more leaks possible.
Motivation:
When a remote peer did open a connection and only do the handshake without sending any data and then directly close the connection we did not call shutdown() in the OpenSslEngine. This leads to a native memory leak. Beside this it also was not fireed when a OpenSslEngine was created but never used.
Modifications:
- Make sure shutdown() is called in all cases when closeInbound() is called
- Call shutdown() also in the finalize() method to ensure we release native memory when the OpenSslEngine is GC'ed
Result:
No more memory leak when using OpenSslEngine
Motivation:
TrafficShapingHandlerTest uses Logback API directly, which is
discouraged. Also, it overrides the global default log level, which
silences the DEBUG messages from other tests.
Modifications:
Remove the direct use of Logback API
Result:
The tests executed after TrafficShapingHandlerTest logs their DEBUG
messages correctly.
Motivation:
There was a bug in the Java ALPN library we are using. A new version was released to fix this bug and we should update our pom.xml to use the new version.
Modifications:
Update pom.xml to use new ALPN library.
Result:
Newer versions of JDK (1.7_u71, 1.7_u72, 1.8_u25) have the bug fixed.
Motivation:
We need more information to understand why SocketSslEchoTest fails
sporadically in the CI machine.
Modifications:
- Refactor SocketSslEchoTest so that it is easier to retrieve the
information about renegotiation and the current progress
Result:
We will get more information when the test fails.
Motivation:
Tests sometimes time out because it took too long to compress the
generated heap dump.
Modifications:
- Move the compression logic to a new method 'compressHeapDumps()'
- Call TestUtils.compressHeapDumps() at the end of the tests, so that
the tests do not fail because of timeout
Result:
JUnit reports the real cause of the test failure instead of timeout
exception.
Modifications:
Converted AsciiString into a String by calling toString() method before comparing with equals(). Also added a unit-test to show that it works.
Result:
Major violation is gone. Code is correct.
Motivation:
without this check then given a URI with path /path the resulting URL will be /path?null=
Modifications:
check that getRawQuery doesn't return null and only append if not
Result:
urls of the form /path will not have a null?= appended
Motivation:
So far, we generated and deployed test JARs to Maven repositories. The
deployed JAR had the classifier 'test-jar'. The test JAR is consumed by
transport-native-epoll as a test dependency.
The problem is, when netty-transport-native-epoll pulls the test JAR as
a dependency, that Maven resolves its transitive dependencies at
'compile' and 'runtime' scope only, which is incorrect.
I was bitten by this problem recently while trying to add a new
dependency to netty-testsuite. Because I added a new dependency at the
'test' scope, the new dependency was not pulled transitively by
transport-native-epoll and caused an unexpected build failure.
- d6160208c3
- bf77bb4c3a
Modifications:
- Move all classes in netty-testsuite from src/test to src/main
- Update the 'compile' scope dependencies of netty-testsuite
- Override the test directory configuration properties of the surefire
plugin
- Do not generate the test JAR anymore
- Update the dependency of netty-transport-native-epoll
Result:
It is less error-prone to add a new dependency to netty-testsuite.
Motivation:
It takes too long to download the heap dump from the CI server.
Modifications:
Compress the heap dump as much as possible.
Result:
When heap dump is generated by certain test failure, the generated heap
dump file is about 3 times smaller than before, although the compression
time will increase the build time when the test fails.
Motivation:
This fixes issue 3168 where HttpObjectAggregator does not suppress
channelReadComplete() when aggregation is not yet finished.
Modifications:
Ignore channelReadComplete until a message completes aggregation.
MessageAggregator currently tracks the currentMessage being aggregated.
This variable transitions to non-null when aggregation begins and back
to null when aggregation completes or fails. When the currentMessage is
null, it is safe to issue a channelReadComplete because the
corresponding channelRead will have completed aggregation.
Result:
channelReadComplete will only fire one time on each completed message
aggregation.
Motivation:
HttpToHttp2ConnectionHandler is awaiting on a future and a latch that may be competed before the buffers actually get released. This test is attempting to validate that the buffer's refCnt() is 0 but there is no mechanism to wait on for a buffer's release() method to be called.
Modifications:
Remove the buffer refCnt() check. The leak profile is designed to pick these up.
Result:
Unit tests that no longer have a race condition.
Motivation:
The stress tests have been observed to fail on the CI server. The average run time of the stress tests has recently been 26+ seconds. Our timeout is currently set to 30 seconds.
Modifications:
Increase the timeout for the stress test so when the leak profile is active we will have more time to complete the test (with the additional overhead).
Result:
Stress tests fail less frequently (hopefully not at all).
Motivation:
Fix a minor documentation bug in
ChannelHandlerContext#fireChannelReadComplete.
Modifications:
ChannelHandlerContext#fireChannelReadComplete no longer references an
incorrect method in its javadoc.
Results:
Documentation is correct.
Motivation:
Everytime a new connection is accepted via EpollSocketServerChannel it will create a new EpollSocketChannel that needs to get the remote and local addresses in the constructor. The current implementation uses new InetSocketAddress(String, int) to create these. This is quite slow due the implementation in oracle and openjdk.
Modifications:
Encode all needed informations into a byte array before return from jni layer and then use new InetSocketAddress(InetAddress, int) to create the socket addresses. This allows to create the InetAddress via a byte[] and so reduce the overhead, this is done either by using InetAddress.getByteAddress(byte[]) or by Inet6Address.getByteAddress(String, byte[], int).
Result:
Reduce performance overhead while accept new connections with native transport
Motivation:
So far, our TLS renegotiation test did not test changing cipher suite
during renegotiation explicitly.
Modifications:
- Switch the cipher suite during renegotiation
Result:
We are now sure the cipher suite change works.
Related:
e9685ea45a
Motivation:
SslHandler.unwrap() does not evaluate the handshake status of
SSLEngine.unwrap() when the status of SSLEngine.unwrap() is CLOSED.
It is not correct because the status does not reflect the state of the
handshake currently in progress, accoding to the API documentation of
SSLEngineResult.Status.
Also, sslCloseFuture can be notified earlier than handshake notification
because we call sslCloseFuture.trySuccess() before evaluating handshake
status.
Modifications:
- Notify sslCloseFuture after the unwrap loop is finished
- Add more assertions to SocketSslEchoTest
Result:
Potentially fix the regression caused by:
- e9685ea45a
Motivation:
We have a few sporadic test failures which are only easily reproduceable
in our CI machine. To get more information about the failure, we need
heap and full thread dump at the moment of failure.
Modifications:
- Add TestUtils.dump() method to dump heap and threads
- Modify SocketGatheringWriteTest and SocketSslEchoTest to call
TestUtils.dump() on failure
Result:
We get more information about the test failure.
Motivation:
Clinker provides a Sonar tool which detects potential bugs or problems in the code. These problems were reported here http://clinker.netty.io/sonar/drilldown/issues/io.netty:netty-parent:master?
Modifications:
Make the recommended changes as reported by Sonar
Result:
Better or more standard code. Less Sonar problem reports for HTTP/2 codec.
Motivation:
We only provided a constructor in DefaultFileRegion that takes a FileChannel which means the File itself needs to get opened on construction. This has the problem that if you want to write a lot of Files very fast you may end up with may open FD's even if they are not needed yet. This can lead to hit the open FD limit of the OS.
Modifications:
Add a new constructor to DefaultFileRegion which allows to construct it from a File. The FileChannel will only be obtained when transferTo(...) is called or the DefaultFileRegion is explicit open'ed via open() (this is needed for the native epoll transport)
Result:
Less resource usage when writing a lot of DefaultFileRegion.
Motivation:
Before we missed to check if a buffer was released before we return the backing byte array or memoryaddress. This could lead to JVM crashes when someone tried various bulk operations on the Unsafe*ByteBuf implementations.
Modifications:
Always check if the buffer is released before all to return the byte array and memoryaddress.
Result:
No more JVM crashes because of released buffers when doing bulk operations on Unsafe*ByteBuf implementations.
Related: #2958
Motivation:
SslHandler currently does not issue a read() request when it is
handshaking. It makes a connection with autoRead off stall, because a
user's read() request can be used to read the handshake response which
is invisible to the user.
Modifications:
- SslHandler now issues a read() request when:
- the current handshake is in progress and channelReadComplete() is
invoked
- the current handshake is complete and a user issued a read() request
during handshake
- Rename flushedBeforeHandshakeDone to flushedBeforeHandshake for
consistency with the new variable 'readDuringHandshake'
Result:
SslHandler should work regardless whether autoRead is on or off.
Related: #3125
Motivation:
We did not expose a way to initiate TLS renegotiation and to get
notified when the renegotiation is done.
Modifications:
- Add SslHandler.renegotiate() so that a user can initiate TLS
renegotiation and get the future that's notified on completion
- Make SslHandler.handshakeFuture() return the future for the most
recent handshake so that a user can get the future of the last
renegotiation
- Add the test for renegotiation to SocketSslEchoTest
Result:
Both client-initiated and server-initiated renegotiations are now
supported properly.
Related: #3212
Motivation:
When SslHandler and ChunkedWriteHandler exists in a pipeline together,
it is possible that ChunkedWriteHandler.channelWritabilityChanged()
invokes SslHandler.flush() and vice versa. Because they can feed each
other (i.e. ChunkedWriteHandler.channelWritabilityChanged() ->
SslHandler.flush() -> ChunkedWriteHandler.channelWritabilityChanged() ->
..), they can fall into an inconsistent state due to reentrance (e.g.
bad MAC record at the remote peer due to incorrect ordering.)
Modifications:
- Trigger channelWritabilityChanged() using EventLoop.execute() when
there's a chance where channelWritabilityChanged() can cause a
reentrance issue
- Fix test failures caused by the modification
Result:
Fix the handler reentrance issues related with a
channelWritabilityChanged() event
Motivation:
The Http2ConnectionRoundtripTest.noMoreStreamIdsShouldSendGoAway unit test had a race condition where it would sometimes receive a SETINGS_ACK message that was not anticipated. This caused the test to fail because of bad test code.
Modifications:
The bad unit test should be updated to handle the message exchange for a good connection setup, and then the GO_AWAY frame.
Result:
Http2ConnectionRoundtripTest.noMoreStreamIdsShouldSendGoAway should no longer sporadically fail.
Related: #3219
Motivation:
ChunkedWriteHandler.flush() does not call ctx.flush() when channel is
not writable. This can be a problem when other handler / non-Netty
thread writes messages simultaneously, because
ChunkedWriteHandler.flush() might have no chance to observe
channel.isWritable() returns true and thus the channel is never flushed.
Modifications:
- Ensure that ChunkedWriteHandler.flush() calls ctx.flush() at least
once.
Result:
A stall connection issue, that occurs when certain combination of
handlers exist in a pipeline, has been fixed. (e.g. SslHandler and
ChunkedWriteHandler)
Motivation:
The terminology used with inbound/outbound is a little confusing since
it's not discussed in the spec. We should switch to using local/remote
instead. Also there is some asymmetry between the inbound/outbound
interfaces which could probably be cleaned up.
Modifications:
Changing the interface names and making a common Http2FlowController
interface for most of the methods.
Result:
The HTTP/2 flow control interfaces should be more clear.
Motivations:
The chunkSize might be oversized after comparison (size being > of int
capacity) if file size is bigger than an integer.
Modifications:
Change it to long.
Result:
There is no more int oversized.
Same fix for 4.1 and Master
Related: #3212
Motivation:
PendingWriteQueue.recycle() updates its data structure after triggering
a channelWritabilityChanged() event. It causes a rare corruption such as
double free when channelWritabilityChanged() method accesses the
PendingWriteQueue.
Modifications:
Update the state of PendingWriteQueue before triggering an event.
Result:
Fix a rare double-free problem
