Related issue: #2735
Motivation:
When an application is under load and experiencing a lot of failure, the
instantiation of DefaultExceptionEvent spends noticeable amount of time
because of StackTraceSimplifier.
Also, StackTraceSimplifier makes people sometimes confused because it
hides the execution path partially.
Modifications:
Remove the use of StackTraceSimplifier
Result:
JVM spends much less time at Throwable.getStackTrace()
Related issue: #2093
Motivation:
When a channel with SslHandler is closed with pending writes (either
unencrypted or encrypted), SslHandler.channelClosed() attempts to
acquire a lock and fail the futures of the pending writes.
If a user added a listener to any of the failed futures and the listener
attempts to write something, it will make SslHandler.handleDownstream()
invoked, which also attempts to acquire the same lock.
Because the lock is non-reentrant, even if these two lock acquisitions
are taking place in the same thread, the second attempt will block for
ever.
Modification:
Do not fail the futures while holding a lock. Notify them after
releasing the lock.
Result:
One less dead lock
Related issue: #2742
Motivation:
When there are more than one stream with the same priority, the set
returned by SpdySession.getActiveStream() will not include all of them,
because it uses TreeSet and only compares the priority of streams. If
two different streams have the same priority, one of them will be
discarded by TreeSet.
Modification:
- Rename getActiveStreams() to activeStreams()
- Replace PriorityComparator with StreamComparator
Result:
Two different streams with the same priority are compared correctly.
Motivation:
When Content-Encoding is deflate or gzip and Content-Length is 0,
there's no need to generate an empty stream (e.g. 20-byte gzip stream).
We can just produce nothing. At least, it works fine with most modern
browsers. Also, we can skip the instantiation of an encoder entirely by
instantiating it lazily.
Modifications:
Backport HttpContentEncoderTest from 4.0
Add similar tests for HttpContentCompressor to ensure the same tests
work with compression.
Result:
Fixes issue #2321
Motivation:
We sometimes get the following exception:
javax.net.ssl.SSLException: SSLEngine is closing/closed
at sun.security.ssl.SSLEngineImpl.kickstartHandshake(SSLEngineImpl.java:692)
at sun.security.ssl.SSLEngineImpl.beginHandshake(SSLEngineImpl.java:734)
at org.jboss.netty.handler.ssl.SslHandler.handshake(SslHandler.java:433)
at org.jboss.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1233)
at org.jboss.netty.handler.ssl.SslHandler.channelDisconnected(SslHandler.java:668)
.. which is triggered when we attempt to issue a handshake even if the
SSLEngine is closed (or being closed).
We don't really need to initiate handshake if:
- SSLEngine is closed already.
- Connection is closed already
Modifications:
Add a boolean parameter to unwrap() and unwrapNonApp() to suppress the
invocation of handshake() when SSLEngine or connection is closed
already.
Motivation:
We forgot to do a null check on the cause parameter of
ChannelFuture.setFailure(cause)
Modifications:
Add a null check
Result:
Fixed issue: #2728
Motivation:
If the requests contains uri parameters but not path the HttpRequestEncoder does produce an invalid uri while try to add the missing path.
Modifications:
Correctly handle the case of uri with paramaters but no path.
Result:
HttpRequestEncoder produce correct uri in all cases.
Backported: d9cccccbb3
Motivation:
Currently it is not possible to load an encrypted private key when
creating a JDK based SSL server context.
Modifications:
- Added static method to JdkSslServerContext which handles key spec
generation for (encrypted) private keys and make use of it.
- Added tests for creating a SSL server context based on a (encrypted)
private key.
Result:
It is now possible to create a JDK based SSL server context with an
encrypted (password protected) private key.
Motivation:
As we cached ConnectTimeoutException we sometimes ended up using the wrong remote address in the exception message.
Modifications:
Always create a new ConnectTimeException and so make sure we use the connect remote address. This has a bit more overhead because of fill in the stacktrace everytime but as this only happens on connection timeouts it should be ok.
Result:
Always include the correct remote address in ConnectTimeoutException.
Motivation:
We forgot to set the flag 'changed' to 'true' after updating
interestOps.
Modifications:
Add a missing 'changed = true;'
Result:
ChannelStateEvent(INTEREST_OPS) is always triggered correctly.
Related issue: #2179 and #2173
Motivation:
When a WebSocket server responds to a WebSocket handshake request with
its first WebSocket frame, it is sometimes swallowed by
HttpMessageDecoder. This issue has been fixed by
4f6ccbbb78 in 4 and 5, but was not.
Modification:
Backport the UPGRADED state and the fix for #2173 to HttpMessageDecoder
Result:
The data sent by the server following an HTTP upgrade response is not
lost anymore.
Motivation:
At the moment the HashedWheelTimer will only remove the cancelled Timeouts once the HashedWheelBucket is processed again. Until this the instance will not be able to be GC'ed as there are still strong referenced to it even if the user not reference it by himself/herself. This can cause to waste a lot of memory even if the Timeout was cancelled before.
Modification:
Add a new queue which holds CancelTasks that will be processed on each tick to remove cancelled Timeouts. Because all of this is done only by the WorkerThread there is no need for synchronization and only one extra object creation is needed when cancel() is executed. For addTimeout(...) no new overhead is introduced.
Result:
Less memory usage for cancelled Timeouts.
Motivation:
When a SSLv2Hello message is received, SSLEngine expects the application buffer size to be more than 30KB which is larger than what SslBufferPool can provide. SSLEngine will always return with BUFFER_OVERFLOW status, blocking the SSL session from continuing the handshake.
Modifications:
When SSLEngine.getSession().getApplicationBufferSize() returns a value larger than what SslBufferPool provides, allocate a temporary heap buffer.
Result:
SSLv2Hello is handled correctly.
Motivation:
When an attribute is ending with an odd number of CR (0x0D), the decoder
add an extra CR in the decoded attribute and should not.
Modifications:
Each time a CR is detected, the next byte was tested to be LF or not. If
not, in a number of places, the CR byte was lost while it should not be.
When a CR is detected, if the next byte is not LF, the CR byte should be
saved as the position point to the next byte (not LF). When a CR is
detected, if there is not yet other available bytes, the position is
reset to the position of CR (since a LF could follow).
A new Junit test case is added, using DECODER and variable number of CR
in the final attribute (testMultipartCodecWithCRasEndOfAttribute).
Result:
The attribute is now correctly decoded with the right number of CR
ending bytes.
Motivation:
According to RFC2616 section 19, boundary string could be quoted, but
currently the PostRequestDecoder does not support it while it should.
Modifications:
Once the boundary is found, one check is made to verify if the boundary
is "quoted", and if so, it is "unqoted".
Note: in following usage of this boundary (as delimiter), quote seems no
more allowed according to the same RFC, so the reason that only the
boundary definition is corrected.
Result:
Now the boundary could be whatever quoted or not. A Junit test case
checks it.
Motivation:
maven-antrun-plugin does not redirect stdin, and thus it's impossible to
run interactive examples such as securechat-client and telnet-client.
org.codehaus.mojo:exec-maven-plugin redirects stdin, but it buffers
stdout and stderr, and thus an application output is not flushed timely.
Modifications:
Deploy a forked version of exec-maven-plugin which flushes output
buffers in a timely manner.
Result:
Interactive examples work. Launches faster than maven-antrun-plugin.
Motivation:
We do not provide such methods that provide the default name of the handlers. It had to be noticed and removed during the review phase, but we failed to do so.
Modifications:
Deprecate getName() static methods in the SOCKS codec classes
Result:
Getting closer to other codecs in API design.
Motivation:
exec-maven-plugin does not flush stdout and stderr, making the console
output from the examples invisible to users
Modification:
Use maven-antrun-plugin instead
Result:
A user sees the output from the examples immediately.
Motivation:
The examples have not been updated since long time ago, showing various
issues fixed in this commit.
Modifications:
- Overall simplification to reduce LoC
- Use system properties to get options instead of parsing args.
- Minimize option validation
- Just use System.out/err instead of Logger
- Do not pass config as parameters - just access it directly
- Move the main logic to main(String[]) instead of creating a new
instance meaninglessly
- Update netty-build-21 to make checkstyle not complain
- Remove 'throws Exception' clause if possible
- Line wrap at 120 (previously at 80)
- Add an option to enable SSL for most examples
- Use ChannelFuture.sync() instead of await()
- Use System.out for the actual result. Use System.err otherwise.
- Delete examples that are not very useful:
- websocket/html5
- websocketx/sslserver
- localecho/multithreaded
- Add run-example.sh which simplifies launching an example from command
line
Result:
Shorter and simpler examples. A user can focus more on what it actually
does than miscellaneous stuff. A user can launch an example very
easily.
Motivation:
According to TLS ALPN draft-05, a client sends the list of the supported
protocols and a server responds with the selected protocol, which is
different from NPN. Therefore, ApplicationProtocolSelector won't work
with ALPN
Modifications:
- Use Iterable<String> to list the supported protocols on the client
side, rather than using ApplicationProtocolSelector
- Remove ApplicationProtocolSelector
Result:
Future compatibility with TLS ALPN
Motivation:
- OpenSslEngine and JDK SSLEngine (+ Jetty NPN) have different APIs to
support NextProtoNego extension.
- It is impossible to configure NPN with SslContext when the provider
type is JDK.
Modification:
- Implement NextProtoNego extension by overriding the behavior of
SSLSession.getProtocol() for both OpenSSLEngine and JDK SSLEngine.
- SSLEngine.getProtocol() returns a string delimited by a colon (':')
where the first component is the transport protosol (e.g. TLSv1.2)
and the second component is the name of the application protocol
- Remove the direct reference of Jetty NPN classes from the examples
- Add SslContext.newApplicationProtocolSelector
Result:
- A user can now use both JDK SSLEngine and OpenSslEngine for NPN-based
protocols such as HTTP2 and SPDY
Motivation:
- In unwrap(), it does not check if the current index of dsts has
reached at its end offset, resulting in IndexOutOfBoundsException.
- SSLEngine does not update the position of the source buffer correctly
when SSL.writeToSSL() returns a negative value.
Modifications:
Fix them all
Result:
Less bugs
Motivation:
Previous fix for the OpenSslEngine compatibility issue (#2216 and
18b0e95659) was to feed SSL records one by
one to OpenSslEngine.unwrap(). It is not optimal because it will result
in more JNI calls.
Modifications:
- Do not feed SSL records one by one.
- Feed as many records as possible up to MAX_ENCRYPTED_PACKET_LENGTH
- Deduplicate MAX_ENCRYPTED_PACKET_LENGTH definitions
Result:
- No allocation of intemediary arrays
- Reduced number of calls to SSLEngine and thus its underlying JNI calls
- A tad bit increase in throughput, probably reverting the tiny drop
caused by 18b0e95659
Motivation:
Although 4cff4b99fd introduced
OpenSslEngine and its helper classes, a user has to write two different
copies of SSL initialization code that does pretty much same job,
because the initialization procedure between JDK SSLEngine and
OpenSslEngine are different.
Modifications:
- Replace OpenSslContextBuilder with SslContext which provides the
unified API for creating an SSL context
- SslContext allows you to create a new SSLEngine or a new SslHandler
with your PKCS#8 key and X.509 certificate chain.
- Merge OpenSslBufferPool into SslBufferPool
- Add an option to preallocate the pool
- Add an option to allocate direct buffers
- When OpenSSL is in use, preallocate direct buffers, which is close
to what OpenSslBufferPool does.
- Add JdkSslContext which is a simple wrapper of JDK's SSLContext
- The specified PKCS#8 key and X.509 certificate chain are converted
to JDK KeyStore in instantiation time.
- Like OpenSslServerContext, it uses sensible default cipher suites now.
- A user does not specify certPath and caPath separately anymore. He or
she has to merge them into a single file. I find this more logical
because previously ca file's first entry and cert file were always same.
- Clean up SSL tests to demonstrate the advantage of this change
- AbstractSocketSsl*Test now uses SslContext.new*Context() to
configure both the client and the server side. We did this only for
the server side previously and had to use different certificates for
JDK SSLEngine and OpenSslEngine, but not anymore.
- Add ApplicationProtocolSelector to ensure the future support for NPN
(NextProtoNego) and ALPN (Application Layer Protocol Negotiation) on
the client-side.
- Add SimpleTrustManagerFactory to help a user write a
TrustManagerFactory easily, which should be useful for those who need
to write an alternative verification mechanism. For example, we can
use it to implement an unsafe TrustManagerFactory that accepts
self-signed certificates for testing purposes.
- Add InsecureTrustManagerFactory and FingerprintTrustManager for quick
and dirty testing
- Add SelfSignedCertificate class which generates a self-signed X.509
certificate very easily.
- Update all our examples to use SslContext.newClient/ServerContext()
- Found that OpenSslEngine performs unnecessary memory copy - optimized
it.
- SslHandler now logs the chosen cipher suite when handshake is
finished.
Result:
- Cleaner unified API for configuring an SSL client and an SSL server
regardless of its internal implementation.
- When native libraries are available, OpenSSL-based SSLEngine
implementation is selected automatically to take advantage of its
performance benefit.
- Examples take advantage of this modification and thus are cleaner.
Motivation:
Some users already use an SSLEngine implementation in finagle-native. It
wraps OpenSSL to get higher SSL performance. However, to take advantage
of it, finagle-native must be compiled manually, and it means we cannot
pull it in as a dependency and thus we cannot test our SslHandler
against the OpenSSL-based SSLEngine. For an instance, we had #2216.
Modifications:
- Pull netty-tcnative in as an optional dependency.
http://netty.io/wiki/forked-tomcat-native.html
- Backport NativeLibraryLoader from 4.0
- Move OpenSSL-based SSLEngine implementation into our code base.
- Copied from finagle-native; originally written by @jpinner et al.
- Overall cleanup by @trustin.
- Run all SslHandler tests with both default SSLEngine and OpenSslEngine
Motivation:
At the moment there are two issues with HashedWheelTimer:
* the memory footprint of it is pretty heavy (250kb fon an empty instance)
* the way how added Timeouts are handled is inefficient in terms of how locks etc are used and so a lot of context-switching / condition can happen.
Modification:
Rewrite HashedWheelTimer to use an optimized bucket implementation to store the submitted Timeouts and a queue to handover the timeouts. So memory foot-print of the buckets itself is reduced a lot as the bucket uses a double-linked-list. Beside this we use Atomic*FieldUpdater where-ever possible to improve the memory foot-print and performance.
Result:
Lower memory-footprint and better performance
