Motivation:
The outbound flow controller currently has to walk the entire tree each
time to calculate the total available data for each subtree. For better
performance we should maintain a running total for each subtree as we
queue/write frames.
Modifications:
I've modified the DefaultHttp2OutboundFlowController to manage the state
of "priorityData" at each node in the priority tree, which is
essentially the total writable data for all streams in that subtree.
These totals do not take into account the connection window, as that is
applied when splitting the data across the streams at each level in the
tree.
The flow controller now sorts the children of a node by the product of
their data (for the subtree) and its weight. This is used since in
certain cases the algorithm might prefer nodes that appear later in the
list. Sorting helps keep nodes with similar characteristics (e.g. a lot
of data and a high priority) with similar output.
To help clean things up, I'm storing a FlowState for the root node as
well, which maintains the runnning total of the currently writable data
for all stream.
Another item of cleanup is that I created a GarbageCollector innerclass
within the outbound flow controller. This keeps all of the garbage
collection code in one place, away from the flow control code.
Also added some more unit tests for the flow controller.
Result:
The outbound flow controller is a bit cleaner and perhaps a bit more
fair when distributing outbound data across streams.
Motivation:
When doing a gathering write we need to update the indices after the write partial completes. In the current code-base we use the wrong value when compare the expected written bytes and the actual written bytes.
Modifications:
Use the correct value when compare.
Result:
Indices are updated correctly and so no corruption can happen when resume writing after data was only partial written before.
Motivation:
Our ci showed some buffer leaks in the http2 codec. This commit fixes all of these.
Modifications:
* Correctly release buffers in DefaultHttp2FrameWriter
* Fix buffer leaks in tests
Result:
Tests pass without leaks
Motivation:
Draft 12 has just arrived and has quite a few changes. Need to update in
order to keep current with the spec.
Modifications:
This is a rewrite of the original (draft 10) code. There are only 2
handlers now: preface and connection. The connection handler is now
callback based rather than frame based (there are no frame classes
anymore). AbstractHttp2ConnectionHandler is the base class for any
HTTP/2 handlers. All of the stream priority logic now resides in the
outbound flow controller, and its interface exposes methods for
adding/updating priority for streams.
Upgraded to hpack 0.7.0, which is used by draft12. Also removed
draft10 code and moved draft12 code to the ../http2 package
(no draft subpackage).
Result:
Addition of a HTTP/2 draft 12 support.
Motivation:
CORS request are currently processed, and potentially failed, after the
target ChannelHandler(s) have been invoked. This might not be desired, for
example a HTTP PUT or POST might have been performed.
Modifications:
Added a shortCurcuit option to CorsConfig which when set will
cause a validation of the HTTP request's 'Origin' header and verify that
it is valid according to the configuration. If found invalid an 403
"Forbidden" response will be returned and not further processing will
take place.
This is indeed no help for non browser request, like using curl, which
can set the 'Origin' header.
Result:
Users can now configure if the 'Origin' header should be validated
upfront and have the request rejected before any further processing
takes place.
Motivation:
Because of not correctly release a buffer before null out the reference a memory leak shows up.
Modifications:
Correct call buffer.release() before null out reference.
Result:
No more leak
Motivation:
Currently there's no way to configure maxFramePayloadSize from
WebSocketServerProtocolHandler, which is the most used entry point of
WebSocket server.
Modifications:
Added another constructor for maxFramePayloadSize.
Result:
We can configure max frame size for websocket packet in
WebSocketServerProtocolHandler. It will also keep backward compatibility
with default max size: 65536. (65536 is hard-coded max size in previous
version of Netty)
Motivation:
DefaultChannelPipeline.firstContext() should return null when the ipeline is empty. This is not the case atm.
Modification:
Fix incorrect check in DefaultChannelPipeline.firstContext() and add unit tests.
Result:
Correctly return null when DefaultChannelPipeline.firstContext() is called on empty pipeline.
Motivation:
oss.sonatype.org refuses to promote an artifact if it doesn't have the
default JAR (the JAR without classifier.)
Modifications:
- Generate both the default JAR and the native JAR to make
oss.sonatype.org happy
- Rename the profile 'release' to 'restricted-release' which reflects
what it really does better
- Remove the redundant <quickbuild>true</quickbuild> in all/pom.xml
We specify the profile 'full' that triggers that property already
in maven-release-plugin configuration.
Result:
oss.sonatype.org is happy. Simpler pom.xml
Motivation:
Netty must be released from RHEL 6.5 x86_64 or compatible so that:
1) we ship x86_64 version of epoll transport officially, and
2) we ensure the ABI compatibility with older GLIBC versions.
The shared library built on a distribution with newer GLIBC will not
run on older distributions.
Modifications:
- When 'release' profile is active, perform an additional check using
maven-enforcer-plugin so that 'mvn release:*' fails when running on
non-RHEL6.5. This rule is active only when releasing, so a user
should not be affected.
- Simplify maven-release-plugin configuration by removing redundant
profiles such as 'linux'. 'linux' is automatically activated when
releasing because we now enforce the release occurs on linux-x86_64.
- Remove the no-osgi profile, which is unused
- Remove the reference to 'sonatype-oss-release' profile in all/pom.xml,
because we always specify 'release' profile when releasing
- Rename the profile 'linux-native' to 'linux' for brevity
- Upgrade oss-parent and maven-enforcer-plugin
Result:
No one can make a mistake to release Netty on an environment that can
produce incompatible or missing native library.
Motivation:
So far, we used a very simple platform string such as linux64 and
linux32. However, this is far from perfection because it does not
include anything about the CPU architecture.
Also, the current build tries to put multiple versions of .so files into
a single JAR. This doesn't work very well when we have to ship for many
different platforms. Think about shipping .so/.dynlib files for both
Linux and Mac OS X.
Modification:
- Use os-maven-plugin as an extension to determine the current OS and
CPU architecture reliable at build time
- Use Maven classifier instead of trying to put all shared libraries
into a single JAR
- NativeLibraryLoader does not guess the OS and bit mode anymore and it
always looks for the same location regardless of platform, because the
Maven classifier does the job instead.
Result:
Better scalable native library deployment and retrieval
Motivation:
Before we aggregated the full text in the WebSocket08FrameDecoder just to fill in the ContinuationWebSocketFrame.aggregatedText(). The problem was that there was no upper-limit and so it would be possible to see an OOME if the remote peer sends a TextWebSocketFrame + a never ending stream of ContinuationWebSocketFrames. Furthermore the aggregation does not really belong in the WebSocket08FrameDecoder, as we provide an extra ChannelHandler for this anyway (WebSocketFrameAggregator).
Modification:
Remove the ContinuationWebSocketFrame.aggregatedText() method and corresponding constructor. Also refactored WebSocket08FrameDecoder a bit to me more efficient which is now possible as we not need to aggregate here.
Result:
No more risk of OOME because of frames.
Motivation:
When writing data from a server before the ssl handshake completes may not be written at all to the remote peer
if nothing else is written after the handshake was done.
Modification:
Correctly try to write pending data after the handshake was complete
Result:
Correctly write out all pending data
Motivation:
Ports range check is not correct
Modification:
Allow port between 0 and 65535. 0 is wildcard / unknown port here
Result:
Correct validation
Motivation:
In the Internet Protocol, the valid port number range is from 1 to 65535
(inclusive on the both side.) However, SocksCmdRequest and SocksCmdResponse
refuses to construct itself when the port number 65535 is specified. Beside
this it excepts 0 as port number which should not allowed.
Modification:
* Not raise an exception when the specified port number is 65535.
* Raise an exception when the specified port number is 0
Result:
Fixes#2428
Motivation:
In the Internet Protocol, the valid port number range is from 1 to 65535
(inclusive on the both side.) However, SocksCmdRequest refuses to
construct itself when the port number 65535 is specified.
Modification:
Do not raise an exception when the specified port number is 65535.
Result:
Fixes#2428
Motivation:
Because Thread.currentThread().interrupt() will unblock Selector.select() we need to take special care when check if we need to rebuild the Selector. If the unblock was caused by the interrupt() we will clear it and move on as this is most likely a bug in a custom ChannelHandler or a library the user makes use of.
Modification:
Clear the interrupt state of the Thread if the Selector was unblock because of an interrupt and the returned keys was 0.
Result:
No more busy loop caused by Thread.currentThread().interrupt()
Motivation:
When CORS has been configured to allow "*" origin, and at the same time
is allowing credentials/cookies, this causes an error from the browser
because when the response 'Access-Control-Allow-Credentials' header
is true, the 'Access-Control-Allow-Origin' must be an actual origin.
Modifications:
Changed CorsHandler setOrigin method to check for the combination of "*"
origin and allowCredentials, and if the check matches echo the CORS
request's 'Origin' value.
Result:
This addition enables the echoing of the request 'Origin' value as the
'Access-Control-Allow-Origin' value when the server has been configured
to allow any origin in combination with allowCredentials.
This allows client requests to succeed when expecting the server to
be able to handle "*" origin and at the same time be able to send cookies
by setting 'xhr.withCredentials=true'. A concrete example of this is
the SockJS protocol which expects behaviour.
Motivation:
At the moment whenever we add/remove a ChannelHandler with an EventExecutorGroup we have two synchronization points in the execution path. One to find the childInvoker and one for add/remove itself. We can eliminate the former by call findIInvoker in the synchronization block, as we need to synchronize anyway.
Modification:
Remove the usage of AtomicFieldUpdater and the extra synchronization in findInvoker by moving the call of the method in the synchronized(this) block.
Result:
Less synchronization points and volatile reads/writes
Motivation:
It is less confusing not to spread Thread.interrupt() calls.
Modification:
- Comments
- Move generatorThread.interrupt() to where currentThread.interrupt() is
triggered
Result:
Code that is easier to read
Motivation:
As discussed in #2250, it will become much less complicated to implement
deregistration and reregistration of a channel once #2250 is resolved.
Therefore, there's no need to deprecate deregister() and
channelUnregistered().
Modification:
- Undeprecate deregister() and channelUnregistered()
- Remove SuppressWarnings annotations where applicable
Result:
We (including @jakobbuchgraber) are now ready to play with #2250 at
master
Motivation:
4 and 5 were diverged long time ago and we recently reverted some of the
early commits in master. We must make sure 4.1 and master are not very
different now.
Modification:
Fix found differences
Result:
4.1 and master got closer.
Motivation:
4 and 5 were diverged long time ago and we recently reverted some of the
early commits in master. We must make sure 4.1 and master are not very
different now.
Modification:
Remove ChannelHandlerInvoker.writeAndFlush(...) and the related
implementations.
Result:
4.1 and master got closer.
Motivation:
4 and 5 were diverged long time ago and we recently reverted some of the
early commits in master. We must make sure 4.1 and master are not very
different now.
Modification:
Remove ChannelHandlerInvokerUtil.invokeWriteAndFlush(...) as it is not used anyway and not exists in 4.1
Result:
4.1 and master got closer.
Motivation:
4 and 5 were diverged long time ago and we recently reverted some of the
early commits in master. We must make sure 4.1 and master are not very
different now.
Modification:
Fix found differences
Result:
4.1 and master got closer.
Motivation:
4 and 5 were diverged long time ago and we recently reverted some of the
early commits in master. We must make sure 4.1 and master are not very
different now.
Modification:
Fix found differences
Result:
4.1 and master got closer.
Motivation:
Due to the complexity of handling deregistration and re-registration of
a channel, we previously decided to remove the deregister() operation
completely to simplify our code. However, we realized that it shouldn't
be that complicated to implement it during our discussion about making
I/O scheduling more flexible and more customizable [1], and thus the
removal of deregistration and re-registration is unnecessary now.
Modification:
- Revert commit c149f4bcc0
- Revert commit e743a27e75
- Make some additional adjustments
Result:
- deregister(), fireChannelUnregistered(), and channelRegistered() were
added back..
- Channel constructors do not require an EventLoop anymore.
[1] https://github.com/netty/netty/issues/2250
Motivation:
When no currentMessage has been set and the channel is inactive, a NPE is raised.
Modification:
Make sure that a currentMessage is available before checking the extras.
Result:
No more NPE raised potentially.
Motivation:
If sun.nio.ch is not optional this will cause troubles in the
OSGi world. The package is not exposed by default in OSGi, so
actually the whole netty framework cannot be used directly.
There are workarounds, but workarounds are ugly. Especially since
the use of sun.nio.ch is optional. So the requirement on the
package should be optional as well.
Modifications:
Make the import of sun.nio.ch optional.
Result:
If the package cannot be imported it will behave as if the package
sun.nio.ch is not present (like with other JVMs). If the package is
exposed in OSGi (e.g. bootclassloader delegation, extension fragment)
it will be used.
Motivation:
I had the NioSocketChannelTest.testFlushCloseReentrance() fail sometimes on one of my linux installation. This change let it pass all the time.
Modification:
Set the SO_SNDBUF to a small value to force split writes
Result:
Test is passing all the time where it was sometimes fail before.
Motivation:
AbstractEpollChannel.clearEpollIn() throws an IllegalStateException if a user tries to change the autoRead configuration for the Channel and the Channel is not registered on an EventLoop yet. This makes it for example impossible to set AUTO_READ to false via the ServerBootstrap as the configuration is modifed before the Channel is registered.
Modification:
Check if the Channel is registered and if not just modify the flags directly so they are respected once the Channel is registered
Result:
It is possible now to configure AUTO_READ via the ServerBootstrap
Motivation:
We are currently try to modify the events via EpollEventLoop even when the channel was closed before and so the fd was set to -1. This fails with a RuntimeException in this case.
Modification:
Always check if the Channel is still open before try to modify the events.
Result:
No more RuntimeException because of a not open channel
Motivation:
At the moment it is not possible to deregister a LocalChannel without close it.
Modification:
Not close the LocalChannel during dergister
Result:
It will be possible in the future to deregister a LocalChannel and register it to another EventLoop
Motivation:
Currently the generics used for TCP_KEEPIDLE, TCP_KEEPINTVL and TCP_KEEPCNT are incorrect.
Modifications:
Use Integer as type
Result:
User can use TCP_KEEPIDLE, TCP_KEEPINTVL and TCP_KEEPCNT as expected
Motivation:
ThreadLocalRandomTest reveals that ThreadLocalRandom's initial seed generation loop becomes tight if the thread is interrupted.
We currently interrupt ourselves inside the wait loop, which will raise an InterruptedException again in the next iteration, resulting in infinite (up to 3 seconds) exception construction and thread interruptions.
Modification:
- When the initial seed generator thread is interrupted, break out of the wait loop immediately.
- Log properly when the initial seed generation failed due to interruption.
- When failed to generate the initial seed, interrupt the generator thread just in case the SecureRandom implementation handles it properly.
- Make the initial seed generator thread daemon and handle potential exceptions raised due to the interruption.
Result:
No more tight loop on interruption. More robust generator thread termination. Fixes#2412
Motivation:
Some SSLEngine implementations violate the contract and raises an
exception when SslHandler feeds an input buffer that contains multiple
SSL records to SSLEngine.unwrap(), while the expected behavior is to
decode the first record and return.
Modification:
- Modify SslHandler.decode() to keep the lengths of each record and feed
SSLEngine.unwrap() record by record to work around the forementioned
issue.
- Rename unwrap() to unwrapMultiple() and unwrapNonApp()
- Rename unwrap0() to unwrapSingle()
Result:
SslHandler now works OpenSSLEngine from finagle-native. Performance
impact remains unnoticeable. Slightly better readability. Fixes#2116.
Motivation:
The problem with the current snappy implementation is that it does
not comply with framing format definition found on
https://code.google.com/p/snappy/source/browse/trunk/framing_format.txt
The document describes that chunk type of the stream identifier is defined
as 0xff. The current implentation uses 0x80.
Modifications:
This patch replaces the first byte of the chunk type of the stream identifier
with 0xff.
Result:
After this modification the snappy implementation is compliant to the
framing format described at
https://code.google.com/p/snappy/source/browse/trunk/framing_format.txt.
This results in a better compatibility with other implementations.
Motivation:
EpollDatagramChannel produced buffer leaks when tried to read from the channel and nothing was ready to be read.
Modifications:
Correctly release buffer if nothing was read
Result:
No buffer leak
Motivation:
Allow to set TCP_KEEPIDLE, TCP_KEEPINTVL and TCP_KEEPCNT in native transport to offer the user with more flexibility.
Modifications:
Expose methods to set these options and write the JNI implementation.
Result:
User can now use TCP_KEEPIDLE, TCP_KEEPINTVL and TCP_KEEPCNT.
Motivation:
Some Android SSLEngine implementations skip FINISHED handshake status
and go straightly into NOT_HANDSHAKING. This behavior blocks SslHandler
from notifying its handshakeFuture, because we do the notification when
SSLEngine enters the FINISHED state.
Modification:
When the current handshake state is NOT_HANDSHAKING and the
handshakeFuture is not fulfilled yet, treat NOT_HANDSHAKING as FINISHED.
Result:
Better Android compatibility - fixes#1823
Motivation:
See: https://github.com/netty/netty/issues/2402
See: https://tools.ietf.org/html/draft-ietf-httpbis-http2-10#section-3.5
Only the client should send the preface string, the server should not.
"The server connection header consists of just a SETTINGS frame (Section 6.5)
that MUST be the first frame the server sends in the HTTP/2 connection."
Modifications:
Split out Http2ClientPrefaceWriter and Http2ServerPrefaceReader from
Http2FrameEncoder and Http2FrameDecoder. The new channel handlers are added
to the pipeline and remove themselves after the preface is written/read.
Result:
HTTP2 client sends preface, server does not, in compliance with spec.
