Related:
- 375b9e1307
Motivation:
Even if a handler called ctx.fireChannelReadComplete(), the next handler
should not get its channelReadComplete() invoked if fireChannelRead()
was not invoked before.
Modifications:
- Ensure channelReadComplete() is invoked only when the handler of the
current context actually produced a message, because otherwise there's
no point of triggering channelReadComplete().
i.e. channelReadComplete() must follow channelRead().
- Fix a bug where ctx.read() was not called if the handler of the
current context did not produce any message, making the connection
stall. Read the new comment for more information.
Result:
- channelReadComplete() is invoked only when it makes sense.
- No stale connection
Motivation:
At the moment we log priming read and handshake errors via info log level and still throw a SSLException that contains the error. We should only log with debug level to generate less noise.
Modifications:
Change logging to debug level.
Result:
Less noise .
Motivation:
Because of a re-entrance bug in PendingWriteQueue it was possible to get the queue corrupted and also trigger an IllegalStateException caused by multiple recycling of the internal PendingWrite objects.
Modifications:
- Correctly guard against re-entrance
Result:
No more IllegalStateException possible
Motivation:
To use WebSocketClientHandshaker / WebSocketServerHandshaker it's currently a requirement of having a HttpObjectAggregator in the ChannelPipeline. This is not a big deal when a user only wants to server WebSockets but is a limitation if the server serves WebSockets and normal HTTP traffic.
Modifications:
Allow to use WebSocketClientHandshaker and WebSocketServerHandshaker without HttpObjectAggregator in the ChannelPipeline.
Result:
More flexibility
Motivation:
Netty uses edge-triggered epoll by default for performance reasons. The downside here is that a messagesPerRead limit can not be enforced correctly, as we need to consume everything from the channel when notified.
Modification:
- Allow to switch epoll modes before channel is registered
- Some refactoring to share more code
Result:
It's now possible to switch epoll mode.
Motiviation:
When using domain sockets on linux it is supported to recv and send file descriptors. This can be used to pass around for example sockets.
Modifications:
- Add support for recv and send file descriptors when using EpollDomainSocketChannel.
- Allow to obtain the file descriptor for an Epoll*Channel so it can be send via domain sockets.
Result:
recv and send of file descriptors is supported now.
Motivation:
Using Unix Domain Sockets can be very useful when communication should take place on the same host and has less overhead then using loopback. We should support this with the native epoll transport.
Modifications:
- Add support for Unix Domain Sockets.
- Adjust testsuite to be able to reuse tests.
Result:
Unix Domain Sockets are now support when using native epoll transport.
Motivation:
SonarQube (clinker.netty.io/sonar) reported a resource which may not have been properly closed in all situations in AbstractDiskHttpData.
Modifications:
- Ensure file channels are closed in the presence of exceptions.
- Correct instances where local channels were created but potentially not closed.
Result:
Less leaks. Less SonarQube vulnerabilities.
Motivation:
SonarQube (clinker.netty.io/sonar) reported a few 'critical' issues related to the OpenSslEngine.
Modifications:
- Remove potential for dereference of null variable.
- Remove duplicate null check and TODO cleanup.
Results:
Less potential for null dereference, cleaner code, and 1 less TODO.
Motivation:
`HttpResponseDecoder` and `HttpRequestDecoder` in the event when the max configured sizes for HTTP initial line, headers or content is breached, sends a `DefaultHttpResponse` and `DefaultHttpRequest` respectively. After this `HttpObjectDecoder` gets into `BAD_MESSAGE` state and ignores any other data received on this connection.
The combination of the above two behaviors, means that the decoded response/request are not complete (absence of sending `LastHTTPContent`). So, any code, waiting for a complete message will have to additionally check for decoder result to follow the correct semantics of HTTP.
If `HttpResponseDecoder` and `HttpRequestDecoder` creates a Full* invalid message then the request/response is a complete HTTP message and hence obeys the HTTP contract.
Modification:
Modified `HttpRequestDecoder`, `HttpResponseDecoder`, `RtspRequestDecoder` and `RtspResponseDecoder` to return Full* messages from `createInvalidMessage()`
Result:
Fixes the wrong behavior of sending incomplete messages from these codecs
Motivation:
SslHandler adds a pending write with an empty buffer and a VoidChannelPromise when a user flush and not pending writes are currently stored. This may produce an IllegalStateException later if the user try to add a ChannelFutureListener to the promise in the next ChannelOutboundHandler.
Modifications:
Replace ctx.voidPromise() with ctx.newPromise()
Result:
No more IllegalStateException possible
Motivation:
At the moment the max number of events that can be handled per epoll wakup was set during construction.
Modifications:
- Automatically increase the max number of events to handle
Result:
Better performance when a lot of events need to be handled without adjusting the code.
Motivation:
The current way how the guard against overflow when generating the nextId() is pretty slow once an overflow happened.
Modifications:
Once a possible overflow is detected all ids used by the EpollEventLoop are scrubed and re-assigned to the registered Channels. This way we only need to do extra work each time an overflow is detected.
Result:
More consistent performance even after the first overflow was detected.
Motivation:
SSLEngine specifies that IllegalArgumentException must be thrown if a null argument is given when using wrap(...) or unwrap(...).
Modifications:
Replace NullPointerException with IllegalArgumentException to match the javadocs.
Result:
Match the javadocs.
Motivation:
We failed to correctly calculate the endOffset when wrap multiple ByteBuffer and so not wrapped everything when an offset > 0 is used.
Modifications:
Correctly calculate endOffset.
Result:
All ByteBuffers are correctly wrapped when offset > 0.
Motivation:
As the ByteBuf is not set to null after release it we may try to release it again in handleReadException()
Modifications:
- set ByteBuf to null to avoid another byteBuf.release() to be called in handleReadException()
Result:
No IllegalReferenceCountException anymore
Motivation:
On Linux, you can gather various metrics using getsockopt(..., TCP_INFO,
...).
Modifications:
Add EpollSocketChannel.tcpInfo() which returns EpollTcpInfo that exposes
all metrics exposed via getsockopt(..., TCP_INFO, ...)
Result:
TCP_INFO support implemented
Motivation:
The JdkZlibDecoder and JZlibDecoder call isReadable and readableBytes in the same method. There is an opportunity to reduce the number of methods calls to just use readableBytes. JdkZlibDecoder reads from a ByteBuf with an absolute index instead of using readerIndex()
Modifications:
- Use readableBytes where isReadable was used
- Correct absolute ByteBuf index to be relative to readerIndex()
Result:
Less method calls duplicating work and preventing an index out of bounds exception.
Motivation:
In the native transport we use getpeername to obtain the remote address from the file descriptor. This may fail for various reasons in which case NULL is returned.
Modifications:
- Check for null when try to obtain remote / local address
Result:
No more NPE
Motivation:
ctx.fireChannelReadComplete() should only be called if something is produced during a channelRead(...) operation. Also we must ensure that it will be called
if channelRead(...) produced something at some point as channelRead(...) maybe called multiple times by the transport before channelReadComplete(...) is called.
Modifications:
- Ensure channelReadComplete(...) only triggers ctx.fireChannelReadComplete() when a previous channelRead(...) call produced a message
- Ensure read() is called of more data is needed
Result:
Correct semantic with channelReadComplete(...) events and also ensure no stales
Motivation:
Internet Explorer doesn't honor Set-Cookie header Max-Age attribute. It only honors the Expires one.
Modification:
Always generate an Expires attribute along the Max-Age one.
Result:
Internet Explorer compatible expiring cookies. Close#1466.
Motivation:
HttpContentDecoder had the following issues:
- For chunked content, the decoder set invalid "Content-Length" header
with length of the first decoded chunk.
- Decoding of FullHttpRequests put both the original conent and decoded
content into output. As result, using HttpObjectAggregator before the
decoder lead to errors.
- Requests with "Expect: 100-continue" header were not acknowleged:
the decoder didn't pass the header message down the handler's chain
until content is received. If client expected "100 Continue" response,
deadlock happened.
Modification:
- Invalid "Content-Length" header is removed; handlers down the chain can either
rely on LastHttpContent message or ask HttpObjectAggregator to add the header.
- FullHttpRequest is split into HttpRequest and HttpContent (decoded) parts.
- Header (HttpRequest) part of request is sent down the chain as soon as it's received.
Result:
The issues are fixed, unittest is added.
Motivation:
HttpPostMultipartRequestDecoder threw an ArrayIndexOutOfBoundsException
when trying to decode Content-Disposition header with filename
containing ';' or protected \\".
See issue #3326 and #3327.
Modifications:
Added splitMultipartHeaderValues method which cares about quotes, and
use it in splitMultipartHeader method, instead of StringUtils.split.
Result:
Filenames can contain semicolons and protected \\".
Motivation:
When SslHandler.unwrap() copies SSL records into a heap buffer, it does
not update the start offset, causing IndexOutOfBoundsException.
Modifications:
- Copy to a heap buffer before calling unwrap() for simplicity
- Do not copy an empty buffer to a heap buffer.
- unwrap(... EMPTY_BUFFER ...) never involves copying now.
- Use better parameter names for unwrap()
- Clean-up log messages
Result:
- Bugs fixed
- Cleaner code
Motivation:
When using OpenSslEngine with the SslHandler it is possible to reduce memory copies by unwrap(...) multiple ByteBuffers at the same time. This way we can eliminate a memory copy that is needed otherwise to cumulate partial received data.
Modifications:
- Add OpenSslEngine.unwrap(ByteBuffer[],...) method that can be used to unwrap multiple src ByteBuffer a the same time
- Use a CompositeByteBuffer in SslHandler for inbound data so we not need to memory copy
- Add OpenSslEngine.unwrap(ByteBuffer[],...) in SslHandler if OpenSslEngine is used and the inbound ByteBuf is backed by more then one ByteBuffer
- Reduce object allocation
Result:
SslHandler is faster when using OpenSslEngine and produce less GC
Motivation:
The SpdyHttpDecoder was modified to support pushed resources that are
divided into multiple frames. The decoder accepts a pushed
SpdySynStreamFrame containing the request headers, followed by a
SpdyHeadersFrame containing the response headers.
Modifications:
This commit modifies the SpdyHttpEncoder so that it encodes pushed
resources in a format that the SpdyHttpDecoder can decode. The encoder
will accept an HttpRequest object containing the request headers,
followed by an HttpResponse object containing the response headers.
Result:
The SpdyHttpEncoder will create a SpdySynStreamFrame followed by a
SpdyHeadersFrame when sending pushed resources.
Motivation:
NioUdtMessageRendezvoudChannelTest.basicEcho() is flakey on Linux and
failing on Windows.
Modifications:
Disable the problematic test until it's fixed.
Result:
Less annoyance
Motivation:
Currently when there are bytes left in the cumulation buffer we do a byte copy to produce the input buffer for the decode method. This can put quite some overhead on the impl.
Modification:
- Use a CompositeByteBuf to eliminate the byte copy.
- Allow to specify if a CompositeBytebug should be used or not as some handlers can only act on one ByteBuffer in an efficient way (like SslHandler :( ).
Result:
Performance improvement as shown in the following benchmark.
Without this patch:
[xxx@xxx ~]$ ./wrk-benchmark
Running 5m test @ http://xxx:8080/plaintext
16 threads and 256 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 20.19ms 38.34ms 1.02s 98.70%
Req/Sec 241.10k 26.50k 303.45k 93.46%
1153994119 requests in 5.00m, 155.84GB read
Requests/sec: 3846702.44
Transfer/sec: 531.93MB
With the patch:
[xxx@xxx ~]$ ./wrk-benchmark
Running 5m test @ http://xxx:8080/plaintext
16 threads and 256 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 17.34ms 27.14ms 877.62ms 98.26%
Req/Sec 252.55k 23.77k 329.50k 87.71%
1209772221 requests in 5.00m, 163.37GB read
Requests/sec: 4032584.22
Transfer/sec: 557.64MB
Motivation:
When a user sees an error message, sometimes he or she does not know
what exactly he or she has to do to fix the problem.
Modifications:
Log the URL of the wiki pages that might help the user troubleshoot.
Result:
We are more friendly.
Motivation:
When a user deliberatively omitted netty-tcnative from classpath, he or
she will see an ugly stack trace of ClassNotFoundException.
Modifications:
Log more briefly when netty-tcnative is not in classpath.
Result:
Better-looking log at DEBUG level
Motivations:
It seems that slicing a buffer and using this slice to write to CTX will
decrease the initial refCnt to 0, while the original buffer is not yet
fully used (not empty).
Modifications:
As suggested in the ticket and tested, when the currentBuffer is sliced
since it will still be used later on, the currentBuffer is retained.
Add a test case for this issue.
Result::
The currentBuffer still has its correct refCnt when reaching the last
write (not sliced) of 1 and therefore will be released correctly.
The exception does no more occur.
This fix should be applied to all branches >= 4.0.
Motivation:
The latest stable RHEL version of 6.x is now 6.6.
Modification:
Update pom.xml's validation configuration
Result:
Can release on the latest stable RHEL version in 6.x
Motivation:
- There's no point of pre-population.
- Waste of memory and time because they are going to be cached lazily
- Some pre-populated cipher suites are ancient and will be unused
Modification:
- Remove cache pre-population
Result:
Sanity restored
Motivation
----------
The performance tests for utf8 also used the getBytes on ASCII,
which is incorrect and also provides different performance numbers.
Modifications
-------------
Use CharsetUtil.UTF_8 instead of US_ASCII for the getBytes calls.
Result
------
Accurate and semantically correct benchmarking results on utf8
comparisons.
When handling an oversized message, HttpObjectAggregator does not wait
until the last chunk is received to produce the failed message, making
AggregatedFullHttpMessage.trailingHeaders() return null.
Related: #3019
Motivation:
We have multiple (Full)HttpRequest/Response implementations and only
some of them implements toString() properly.
Modifications:
- Add the reusable string converter for HttpMessages to HttpMessageUtil
- Implement toString() of (Full)HttpRequest/Response implementations
properly using HttpMessageUtil
Result:
Prettier string representation is returned by HttpMessage
implementations.
Related: #3274
Motivation:
channelReadComplete() event is not triggered after reading successfully
in EpollDatagramChannel.
Modifications:
- Trigger exceptionCaught() event for read failure only once for less
noise
- Trigger channelReadComplete() event at the end of the read.
Result:
Fix#3274
Motivation:
Calling JNI methods is pretty expensive, so we should only do if needed.
Modifications:
Lazy call methods if needed.
Result:
Better performance.
Motivation:
SSL_set_cipher_list() in OpenSSL does not fail as long as at least one
cipher suite is available. It is different from the semantics of
SSLEngine.setEnabledCipherSuites(), which raises an exception when the
list contains an unavailable cipher suite.
Modifications:
- Add OpenSsl.isCipherSuiteAvailable(String) which checks the
availability of a cipher suite
- Raise an IllegalArgumentException when the specified cipher suite is
not available
Result:
Fixed compatibility
