Motivation:
When trying to write more then Integer.MAX_VALUE / SSIZE_MAX via writev(...) the OS may return EINVAL depending on the kernel or the actual OS (bsd / osx always return EINVAL). This will trigger an IOException.
Modifications:
Never try to write more then Integer.MAX_VALUE / SSIZE_MAX when using writev.
Result:
No more IOException when write more data then Integer.MAX_VALUE / SSIZE_MAX via writev.
Motivation:
We mitigate callouts to System.nanoTime() in SingleThreadEventExecutor
as it is 'relatively expensive'. On a modern system, tak translates to
about 20ns per call. With channelReadComplete() we can side-step this in
channelRead().
Modifications:
Introduce a boolean flag, which indicates that a read batch is currently
on-going, which acts as a flush guard for lastReadTime. Update
lastReadTime in channelReadComplete() just before setting the flag to
false. We set the flag to true in channelRead().
The periodic task examines the flag, and if it observes it to be true,
it will reschedule the task for the full duration. If it observes as
false, it will read lastReadTime and adjust the delay accordingly.
Result:
ReadTimeoutHandler calls System.nanoTime() only once per read batch.
Motivation:
The PooledByteBufAllocator is more or less a black-box atm. We need to expose some metrics to allow the user to get a better idea how to tune it.
Modifications:
- Expose different metrics via PooledByteBufAllocator
- Add *Metrics interfaces
Result:
It is now easy to gather metrics and detail about the PooledByteBufAllocator and so get a better understanding about resource-usage etc.
Motivation:
The Http2OutboundFrameLogger logs all PING frames as not acks.
Modifications:
Changed the logger to correctly log PING acks.
Result:
PING acks are logged correctly.
Motivation:
At the moment when calling slice(...) or duplicate(...) on a Pooled*ByteBuf a new SlicedByteBuf or DuplicatedByteBuf. This can create a lot of GC.
Modifications:
Add PooledSlicedByteBuf and PooledDuplicatedByteBuf which will be used when a PooledByteBuf is used.
Result:
Less GC.
Motivation:
From the javadocs of ByteBuf.duplicate() it is not clear if the reader and writer marks will be duplicated.
Modifications:
Add sentence to clarify that marks will not be duplicated.
Result:
Clear semantics.
Motivation:
When allocate a PooledByteBuf we need to ensure to also reset the markers for the readerIndex and writerIndex.
Modifications:
- Correct reset the markers
- Add test-case for it
Result:
Correctly reset markers.
Motiviation:
When tried to allocate tiny and small sized and failed to serve these out of the PoolSubPage we exit the synchronization
block just to enter it again when call allocateNormal(...).
Modification:
Not exit the synchronized block until allocateNormal(...) is done.
Result:
Better performance.
Motivation:
Adding an example that showcases Netty’s HTTP/2 codec and that is
slightly more complex than the existing hello-world example. It is
based on the Gopher tiles example available here:
https://http2.golang.org/gophertiles?latency=0
Modifications:
Moved current http2 example to http2/helloworld.
Added http2 tiles example under http2/tiles.
Result:
A Netty tiles example is available.
Motiviation:
Interface changes between master and 4.1 branch resulted in a compile failure.
Modifications:
- change messageReceived to channelRead0
Result:
No more compile error.
Motiviation:
The HTTP/2 server example just hangs when a client is using only HTTP with no ALPN or upgrade attempts. We should still send some kind of response.
Modifications:
The HTTP/2 server example has a special handler to detect no upgrade HTTP clients and generate a response.
Result:
Clients that just use HTTP with no upgrade will no appear hung when interacting with the HTTP/2 server example.
Motivation:
At the moment hostname verification is not supported with OpenSSLEngine.
Modifications:
- Allow to create OpenSslEngine with peerHost and peerPort informations.
- Respect endPointIdentificationAlgorithm and algorithmConstraints when set and get SSLParamaters.
Result:
hostname verification is supported now.
Motiviation:
The Http2ConnectionHandler is incrementing the reference count in the goAway method for the debugData buffer after it has already been sent and maybe consumed. This may result in an IllegalRefCountException to be thrown. The unit tests also encounter buffer leaks because they have not been updated to invoke the listener which releases the buffer in the goAway method.
Modifications:
- The retain() call should be before the frameWriter().writeGoAway(...) call
- The unit tests which call goAway must also invoke the operationComplete(..) method for the listener.
Result:
No IllegalRefCountException. Less buffer leaks in tests.
Motivation:
* Path attribute should be null, not empty String, if it's passed as "Path=".
* Only extract attribute value when the name is recognized.
* Only extract Expires attribute value String if MaxAge is undefined as it has precedence.
Modification:
Modify ClientCookieDecoder.
Add "testIgnoreEmptyPath" test in ClientCookieDecoderTest.
Result:
More idyomatic Path behavior (like Domain).
Minor performance improvement in some corner cases.
Motivation:
keyManager() is required on server-side, and so there is a forServer()
method for each override of keyManager(). However, one of the
forServer() overrides was missing, which meant that if you wanted to use
a KeyManagerFactory you were forced to provide garbage configuration
just to get past null checks.
Modifications:
Add missing override.
Result:
No hacks to use SslContextBuilder on server-side with KeyManagerFactory.
Resolves#3775
Motivation:
New versions of compression libraries, which improve their performance and fix some bugs.
Modifications:
Updated versions of jzlib, compress-lzf, lz4 and commons-compres libraries.
Result:
Better stability and performance of compression codecs.
Motivations:
When using HttpPostRequestEncoder and trying to set an attribute if a
charset is defined, currenlty implicit Charset.toStrng() is used, given
wrong format.
As in Android for UTF-16 = "com.ibm.icu4jni.charset.CharsetICU[UTF-16]".
Modifications:
Each time charset is used to be printed as its name, charset.name() is
used to get the canonical name.
Result:
Now get "UTF-16" instead.
(3.10 version)
Motivation:
There is an error in the ByteString test logic which is resulting in test failures.
Modifications:
- Fix the loop iteration to use the loop iteration variable instead of a fixed index.
Result:
Tests are less buggy.
Motivation:
Examples that are using ALPN/NPN are using a failure mode which is not supported by the JDK SslProvider. The examples fail to run and throw an exception if the JDK SslProvider is used.
Modifications:
- Use SelectorFailureBehavior.NO_ADVERTISE
- Use SelectedListenerFailureBehavior.ACCEPT
Result:
Examples can be run with both OpenSsl and JDK SslProviders.
Motivation:
Using factory methods of SslContext is deprecated. Code should be using
SslContextBuilder instead. This would have been done when the old
methods were deprecated, but memcache and http2 examples didn't exist in
the 4.0 branch which the PR was against.
Modifications:
Swap to the new construction pattern.
Result:
No more deprecated warnings during build of examples. Users are
instructed to use the new pattern.
Motivation:
If headers are sent on a stream that does not yet exist and the END_STREAM flag is set we will send a RST_STREAM frame. We should send the HEADERS frame and no RST_STREAM.
Modifications:
DefaultHttp2RemoteFlowController should allow frames to be sent if stream is created in the 'half closed (local)' state.
Result:
We can send HEADERS frame with the END_STREAM flag sent without sending a RST_STREAM frame.
Motivation:
There were a few outstanding comments that were left unaddressed after committing the changes for #3749.
Modifications:
Changes to Http2ConnectionHandler.goAway():
- Retaining the debugData buffer, rather than always converting it to a string immediately.
- Changing log level for sending a GOAWAY with error to debug.
Result:
Remaining comments from #3749 are addressed.
Motivation:
To prevent from DOS attacks it can be useful to disable remote initiated renegotiation.
Modifications:
Add new flag to OpenSslContext that can be used to disable it
Adding a testcase
Result:
Remote initiated renegotion requests can be disabled now.
Motivation:
The way of firstIndexOf and lastIndexOf iterating the ByteBuf is similar to forEachByte and forEachByteDesc, but have many range checks.
Modifications:
Use forEachByte and a IndexOfProcessor to find occurrence.
Result:
eliminate range checks
RFC6265 specifies which characters are allowed in a cookie name and value.
Netty is currently too lax, which can used for HttpOnly escaping.
Modification:
In ServerCookieDecoder: discard cookie key-value pairs that contain invalid characters.
In ClientCookieEncoder: throw an exception when trying to encode cookies with invalid characters.
Result:
The problem described in the motivation section is fixed.
Motivation:
NioUdtByteAcceptorChannel and NioUdtMessageAcceptorChannel have almost same code.
For maintainability, it's better to remove it.
Motification:
- Pulled a member(METADATA) and methods(doReadMessage() and metadata() up.
- Added newConnectorChannel().
Result:
Cleaner code.
Motivation:
Currently the graceful shutdown of the HTTP/2 connection waits until there are no active streams. There may be use cases that buffer stream creation (due to limits imposed by MAX_CONCURRENT_STREAMS), in which case they may still want those streams to complete before closing.
Modifications:
Added a isGracefulShutdownComplete method to Http2ConnectionHandler, which can be overridden by a subclass.
Result:
Graceful shutdown logic can be overridden.
Motivation:
In the SslHandler we schedule a timeout at which we close the Channel if a timeout was detected during close_notify. Because this can race with notify the flushFuture we can see an IllegalStateException when the Channel is closed.
Modifications:
- Use a trySuccess() and tryFailure(...) to guard against race.
Result:
No more race.
Motivation:
We should not trigger channelWritabilityChanged during failing message when we are about to close the Channel as otherwise the use may try again writing even if the Channel is about to get closed.
Modifications:
Add new boolean param to ChannelOutboundBuffer.failFlushed(...) which allows to specify if we should notify or not.
Result:
channelWritabilityChanged is not triggered anymore if we cloe the Channel because of an IOException during write.
Motivation:
Previously, we deferred the closing of the Channel when we were flushing. This is problematic as this means that if the user adds a ChannelFutureListener, that will close the Channel, the closing will not happen until we are done with flushing. This can lead to more data is sent than expected.
Modifications:
- Do not defer closing when in flush
Result:
Correctly respect order of events and closing the Channel ASAP
Motivation:
channelUDT() can't handle NioUdtByteRendezvousChannel and NioUdtMessageRendezvousChannel because those are handled by the checking condition of their parent.
Motification:
Reorder checking conditions.
Result:
Bugfixed.
Motivation:
The semantic of LocalChannel.doWrite(...) were a bit off as it notified the ChannelFuture before the data was actual moved to the peer buffer.
Modifications:
- Use our MPSC queue as inbound buffer
- Directly copy to data to the inbound buffer of the peer and either success or fail the promise after each copy.
Result:
Correct semantic and less memory copies.
Motivation:
Currently mutual auth is not supported when using OpenSslEngine.
Modification:
- Add support to OpenSslClientContext
- Correctly throw SSLHandshakeException when an error during handshake is detected
Result:
Mutual auth can be used with OpenSslEngine
Motivation:
When EPOLLRDHUP is received we need to try to read at least one time to ensure
that we read all pending data from the socket. Otherwise we may loose data.
Modifications:
- Ensure we read all data from socket
- Ensure file descriptor is closed on doClose() even if doDeregister() throws an Exception.
- Only handle either EPOLLRDHUP or EPOLLIN as only one is needed to detect connection reset.
Result:
No more data loss on connection reset.
Motivation:
When using epoll_ctl we should respect the return value and do the right thing depending on it.
Modifications:
Adjust java and native code to respect epoll_ctl return values.
Result:
Correct and cleaner code.
Motiviation:
If user events or excpetions reach the tail end of the pipeline they are not released. This could result in buffer leaks.
Motivation:
- Use the ReferenceCountUtil.release to release objects for the userEventTriggered and exceptionCaught methods on DefaultChannelPipeline
Result:
2 less areas where buffer leaks can occur.
Motivation:
Because we tried to grab the SSL renegotation future to early we could see test-failures.
Modifications:
Access the future at the correct time.
Result:
No more test-failures.
Motivation:
All read operations should be safe to execute from multiple threads which was not the case and so could produce a livelock.
Modifications:
Modify methods so these are safe to be called from multiple threads.
Result:
No more livelock.
Motivation:
If the client closes, a GOWAY is sent with a lastKnownStream of zero (since the remote side never created a stream). If there is still an exchange in progress, inbound frames for streams created by the client will be ignored because our ignore logic doesn't check to see if the stream was created by the remote endpoint. Frames for streams created by the local endpoint should continue to come through after sending GOAWAY.
Modifications:
Changed the decoder's streamCreatedAfterGoAwaySent logic to properly ensure that the stream was created remotely.
Result:
We now propertly process frames received after sending GOAWAY.
Motivation:
The isDone method is currently broken in the aggregator because the doneAllocatingPromises accidentally calls the overridden version of setSuccess, rather than calling the base class version. This causes the base class's version to never be called since allowNotificationEvent will evaluate to false. This means that setSuccess0 will never be set, resulting in isDone always returning false.
Modifications:
Changed setSuccess() to call the base class when appropriate, regardless of the result of allowNotificationEvent.
Result:
isDone now behaves properly for the promise aggregator.
