Motivation:
We need to ensure we not write any body when a response with status code of 1xx, 204 or 304 is used as stated in rfc:
https://tools.ietf.org/html/rfc7230#section-3.3.3
Modifications:
- Correctly handle status codes
- Add unit tests
Result:
Correctly handle responses with 1xx, 204, 304 status codes.
Motivation:
NativeLibraryLoader uses ClassLoader#getResource method that can return nulls when the resource cannot be found. The returned url variable should be checked for nullity and fail in a more usable manner than a NullPointerException
Modifications:
Fail with a FileNotFoundException
Result:
Fixes [#7222].
Motivation:
The constrcutors a protected atm but the classes are public. We should make the constructors public as well to make it easier to write your own ByteBufAllocator.
Modifications:
Change constructors to be public and add some javadocs.
Result:
Easier to create own ByteBufAllocator.
Motiviation:
The ResourceLeakDetector helps to detect and troubleshoot resource leaks and is often used even in production enviroments with a low level. Because of this its import that we try to keep the overhead as low as overhead. Most of the times no leak is detected (as all is correctly handled) so we should keep the overhead for this case as low as possible.
Modifications:
- Only call getStackTrace() if a leak is reported as it is a very expensive native call. Also handle the filtering and creating of the String in a lazy fashion
- Remove the need to mantain a Queue to store the last access records
- Add benchmark
Result:
Huge decrease of performance overhead.
Before the patch:
Benchmark (recordTimes) Mode Cnt Score Error Units
ResourceLeakDetectorRecordBenchmark.record 8 thrpt 20 4358.367 ± 116.419 ops/s
ResourceLeakDetectorRecordBenchmark.record 16 thrpt 20 2306.027 ± 55.044 ops/s
ResourceLeakDetectorRecordBenchmark.recordWithHint 8 thrpt 20 4220.979 ± 114.046 ops/s
ResourceLeakDetectorRecordBenchmark.recordWithHint 16 thrpt 20 2250.734 ± 55.352 ops/s
With this patch:
Benchmark (recordTimes) Mode Cnt Score Error Units
ResourceLeakDetectorRecordBenchmark.record 8 thrpt 20 71398.957 ± 2695.925 ops/s
ResourceLeakDetectorRecordBenchmark.record 16 thrpt 20 38643.963 ± 1446.694 ops/s
ResourceLeakDetectorRecordBenchmark.recordWithHint 8 thrpt 20 71677.882 ± 2923.622 ops/s
ResourceLeakDetectorRecordBenchmark.recordWithHint 16 thrpt 20 38660.176 ± 1467.732 ops/s
Motivation:
When SO_LINGER is used we run doClose() on the GlobalEventExecutor by default so we need to ensure we schedule all code that needs to be run on the EventLoop on the EventLoop in doClose. Beside this there are also threading issues when calling shutdownOutput(...)
Modifications:
- Schedule removal from EventLoop to the EventLoop
- Correctly handle shutdownOutput and shutdown in respect with threading-model
- Add unit tests
Result:
Fixes [#7159].
Motivation:
c93e58c453 changed to use _ for the tcnative lib name but missed to also adjust the filtering.
Modifications:
Fix filtering to look for _
Result:
Not include native tcnative lib as expected.
Motivation:
A large frame will be componsed by many packages. Every time the package
arrived, findEndOfLine will be called from the start of the buffer. It
will cause the complexity of reading frame equal to O(n^2). This can be
eliminated by using a offset to mark the last scan position, when new
package arrived, just find the delimter from the mark. The complexity
will be O(n).
Modification:
Add a offset to mark the last scan position.
Result:
Better performance for read large frame.
Motiviation:
At the moment an NPE is thrown if someone tries to use the InboundHttp2ToHttpAdapter.
Modifications:
- Ensure the status was null in "InboundHttp2ToHttpAdapter::onPushPromiseRead" before calling "HttpConversionUtil.parseStatus" methods.
- Fix setting status to OK in "InboundHttp2ToHttpAdapter::onPushPromiseRead".
Result:
Fixes [#7214].
Motivation:
The Headers interface supports an interface to get all the headers values corresponding to a particular name. This API returns a List which requires intermediate storage and increases GC pressure.
Modifications:
- Add a method which returns an iterator over all the values for a specific name
Result:
Ability to iterator over values for a specific name with no intermediate collection.
Motivation:
RLD allocates an ArrayDeque in anticipation of recording access
points. If the leak detection level is less than ADVANCED though,
the dequeue is never used. Since SIMPLE is the default level,
there is a minor perf win to not preemptively allocate it.
This showed up in garbage profiling when creation a high number of
buffers.
Modifications:
Only allocate the dequeue if it will be used.
Result:
Less garbage created.
Motivation:
NativeLibraryLoader may only log a debug statement if the library is successfully loaded from java.library.path, but will log failure statements the if load for java.library.path fails which can mislead users to believe the load actually failed when it may have succeeded.
Modifications:
- Always load a debug statement when a library was successfully loaded
Result:
NativeLibraryLoader log statements more clear.
Motivation:
When Log4j2Logger is used with PatternLayout (%F:%L)%c.%M, the log message incorrect shows:
(Log4J2Logger.java:73)io.netty.util.internal.PlatformDependent0.debug ....
Modification:
Extend AbstractLogger
Result:
Fixes [#7186].
Motivation:
transport-rxtx has no tests and there is really no easy way to add some. Beside this this transport is not really well maintained.
Modifications:
Mark transport-rxtx as @deprecated so we can drop it in next major version.
Result:
Notify users of plan to drop the transport.
Motivation:
The MQTT decoder should raise an exception trying to build a CONNECT packet where password field is set but not the username one (as by MQTT 3.1/3.1.1 spec).
Modification:
Throw exception if password field is set but not the username
Result:
Fixes [#7205].
Motivation:
We need to ensure we use the correct osname in the Bundle-NativeCode declaration as declared in:
https://www.osgi.org/developer/specifications/reference/
Modifications:
Update osname to match the spec.
Result:
Correct Bundle-NativeCode entry in the MANIFEST
Motivation:
We missed to mark the Http2StreamChannel as writable in some cases which could lead to the situation that a Channel never becomes writable. Also when a Http2StreamChannel was created we always marked it non-writable at the beginning which means if the user will only start writing once the Channel becomes writable it will never happen as it only became writable after the first header was written.
Modifications:
- Correctly handle updates for writability in all cases
- Change unit tests to cover this.
Result:
Fixes [#7179].
Motiviation:
At the moment an NPE is thrown if someone tries to use the Http2ServerUpgradeCodec with Http2FrameCodec and Http2MultiplexCodec.
Modifications:
- Ensure the handler was added to the pipeline before calling on*Upgrade(...) methods.
- Add tests
- Fix adding of handlers after upgrade.
Result:
Fixes [#7173].
Motivation:
In `Http2StreamChannelId` a `hashCode()` is not consistent with `equals()`.
Modifications:
Make a `Http2StreamChannelId.hashCode()` consistent with `equals()`.
Result:
Faster hash map's operations where the Http2StreamChannelId as keys.
Motivation:
A `DefaultChannelId` has final `hashCode` field calculated in the constructor. We can use it in `equals` to the fast return for different objects.
Modifications:
Use `hashCode` field in `DefaultChannelId.equals()`.
Result:
Fast `equals` on negative scenarios.
Motivation:
`useCacheForAllThreads` may be false which disables memory caching
on non netty threads. Setting this argument or the system property
makes it impossible to use `PooledByteBufAllocator`.
Modifications:
Delayed the check of `freeSweepAllocationThreshold` in
`PoolThreadCache` to after it knows there will be any caches in
use. Additionally, check if the caches will have any data in them
(rather than allocating a 0-length array).
A test case is also added that fails without this change.
Results:
Fixes#7194
Motivation:
We recently saw an assertion failure when running DatagramUnicastTest.testSimpleSendWithConnect.
Modifications:
- Adding more debug infos
- Ensure we always correctly release the buffers.
Result:
More informations when tests fail.
Motivation:
We should not log by default if the promise is a VoidChannelPromise as its try* methods will always return false.
Modifications:
Do an instanceof check to determine if we should log or not by default
Result:
No more noise in the logs when using a VoidChannelPromise.
Motivation:
In `DefaultHttp2GoAwayFrame.equals()` a content compared twice: explicitly and in the `super` method.
Modifications:
Remove explicit content comparision.
Make `hashCode()` consistent with `equals()`.
Result:
A `DefaultHttp2GoAwayFrame.equals()` work faster.
Motivation:
We must not add the inboundStreamHandler for outbound streams creates by Http2MultiplexCodec as the user will specify a handler via Http2StreamChannelBootstrap.
Modifications:
- Check if the stream is for outbound and if so not add the inboundStreamHandler to the pipeline
- Update tests so this is covered.
Result:
Fixes [#7178]
Motivation:
379ac890f4 introduced the usage of the inline mock maker. This unfortunally not work on java7 and java9.
Modifications:
Just use reflection to create the event for now.
Result:
Netty tests pass again on java7 and java9 as well.
Motivation:
We should only try to load the native artifacts if the architecture we are currently running on is the same as the one the native libraries were compiled for.
Modifications:
Include architecture in native lib name and append the current arch when trying to load these. This will fail then if its not the same as the arch of the compiled arch.
Result:
Fixes [#7150].
Motivation:
When using Http2FrameCodec / Http2MultiplexCodec with HttpServerUpgradeHandler reference count exception will be triggered.
Modifications:
- Correctly retain before calling InboundHttpToHttp2Adapter.handle
- Add unit test
Result:
Fixes [#7172].
Motivation:
When the user want to have the direct memory explicitly managed by the GC (just as java.nio does) it is useful to be able to construct an UnpooledByteBufAllocator that allows this without the chances to see any memory leak.
Modifications:
Allow to explicitly disable the usage of reflection to construct direct ByteBufs and so be sure these will be collected by GC.
Result:
More flexible way to use the UnpooledByteBufAllocator.
Motivation:
HttpObjectEncoder allocates a new buffer when encoding the initial line and headers, and also allocates a buffer when encoding the trailers. The allocation always uses the default size of 256. This may lead to consistent under allocation and require a few resize/copy operations which can cause GC/memory pressure.
Modifications:
- Introduce a weighted average which tracks the historical size of encoded data and uses this as an estimate for future buffer allocations
Result:
Better approximation of buffer sizes.
Motivation:
The documentation for field updates says:
> Note that the guarantees of the {@code compareAndSet}
> method in this class are weaker than in other atomic classes.
> Because this class cannot ensure that all uses of the field
> are appropriate for purposes of atomic access, it can
> guarantee atomicity only with respect to other invocations of
> {@code compareAndSet} and {@code set} on the same updater.
This implies that volatiles shouldn't use normal assignment; the
updater should set them.
Modifications:
Use setter for field updaters that make use of compareAndSet.
Result:
Concurrency compliant code
Motivation:
By STOMP 1.2 specification - header name or value include any octet except CR or LF or ":".
Modification:
Add constructor argument that allows to enable / disable validation.
Result:
Fixes [#7083]
Motivation:
PD and PD0 Both try to find and use Unsafe. If unavailable, they
try to log why and continue on. However, it is not always east to
enable this logging. Chaining exceptions together is much easier
to reach, and the original exception is relevant when Unsafe is
needed.
Modifications:
* Make PD log why PD0 could not be loaded with a trace level log
* Make PD0 remember why Unsafe wasn't available
* Expose unavailability cause through PD for higher level use.
* Make Epoll and KQueue include the reason when failing
Result:
Easier debugging in hard to reconfigure environments
Motivation:
According to SOCKS 5 spec, dstPort = 0 is a valid value in case of UDP ASSOCIATE.
Modifications:
- Allow 0 as port.
- Add unit tests.
Result:
Fixes [#7156].
Motivation:
ServerCookieEncoder’s javadoc contains some invalid copy-pasting from
ClientCookieEncoder.
Modifications:
* As per RFC6265, multiple cookies are sent as separate Set-Cookie
response headers.
* Fix code sample
Result:
Proper javadoc
Motivation:
The decode method is too large to be inlined with default compiler settings, hence the uncommon paths need to be packed and moved away form the common one.
Modifications:
The uncommon paths of the decode call (eg failures with thrown exceptions) are packed and moved in private methods in order to reduce the size of the common one
and let it being inlined.
Result:
The decode method is being inlined if the stack depth allows it.
Motivation:
If AutoClose is false and there is a IoException then AbstractChannel will not close the channel but instead just fail flushed element in the ChannelOutboundBuffer. AbstractChannel also notifies of writability changes, which may lead to an infinite loop if the peer has closed its read side of the socket because we will keep accepting more data but continuously fail because the peer isn't accepting writes.
Modifications:
- If the transport throws on a write we should acknowledge that the output side of the channel has been shutdown and cleanup. If the channel can't accept more data because it is full, and still healthy it is not expected to throw. However if the channel is not healthy it will throw and is not expected to accept any more writes. In this case we should shutdown the output for Channels that support this feature and otherwise just close.
- Connection-less protocols like UDP can remain the same because the channel may disconnected temporarily.
- Make sure AbstractUnsafe#shutdownOutput is called because the shutdown on the socket may throw an exception.
Result:
More correct handling of write failure when AutoClose is false.
Motivation:
ShutdownOutput now fails all pending writes in the ChannelOutboundBuffer and sets it to null. However the Close code path uses the ChannelOutboundBuffer as an indication that the close operation is in progress and exits early and will not call doClose. This will lead to the Channel not actually being fully closed.
Bug introduced by 237a4da1b7
Modifications:
- AbstractChannel#close shouldn't exit early just because outboundBuffer is null, and instead should use additional state closeInitiated to avoid duplicate close operations
Result:
AbstractChannel#close(..) after AbstractChannel#shutdownOutbound() will still invoke doClose and cleanup Channel state.
Motivation:
Netty is unable to use Java9s ALPN support atm.
Modifications:
When running on Java9+ we invoke the correct methods that are exposed on the Java9+ implementation of SSLEngine and so be able to support ALPN.
This patch is based on the work of @rschmitt and so https://github.com/netty/netty/pull/6992.
Result:
Fixes#6933.
Motivation:
netty-tcnative recently change the name of the native libraries from using - to _.
Modifications:
- OpenSsl should use _ instead of - even for the classifiers to be consistent with netty-tcnative
Result:
Loading netty-tcnative works.
Motivation:
We should deprecate ApplicationProtocolNegotiator as the users should use ApplicationProtocolConfig these days.
Modifications:
Add deprecation annotations and javadocs.
Result:
Be able to make package-private in next major release.
Motivation:
We should try to bind to an ipv6 only socket before we enable ipv6 support in the native transport as it may not work due setup of the platform.
Modifications:
Try to bind to ::1 use IPV6 later on if this works
Result:
Fixes [#7021].
Motivation:
Continuing to make netty happy when compiling through errorprone.
Modification:
Mostly comments, some minor switch statement changes.
Result:
No more compiler errors!
Motivation:
When SslHandlerTest#testCompositeBufSizeEstimationGuaranteesSynchronousWrite fails it would be useful to know the SslProvider type
Modifications:
- Print the sever and client SslProvider upon failure
- Increase test timeout to 8 minutes to allow more time to run
Result:
Failures include more info to help diagnose issues.
Motivation:
DnsCache (an interface) is coupled to DnsCacheEntry (a final class). This means that DnsCache implementations can't implement their own DnsCacheEntry objects if the default behavior isn't appropriate.
Modifications:
- DnsCacheEntry should be moved to DefaultDnsCache as it is an implementation detail
- DnsCache#cache(..) should return a new DnsCacheEntry
- The methods which from DnsCacheEntry that were used outside the scope of DefaultDnsCache should be moved into an interface
Result:
DnsCache is more extensible and not tightly coupled to a default implementation of DnsCacheEntry.
Motivation:
DelegatingSslContext at the moment intercept newEngine calls and allow to init the SslEngine after it is created. The problem here is that this may not work the SSLEngine that is wrapped in the SslHandler when calling newHandler(...). This is because some SslContext implementations not delegate to newEngine(...) when creating the SslHandler to allow some optimizations. For this we should also allow to init the SslHandler after its creation and by default just delegate to initEngine(...).
Modifications:
Allow the user to also init the SslHandler after creation while by default init its SSLEngine after creation.
Result:
More flexible and correct code.
