Commit Graph

8417 Commits

Author SHA1 Message Date
Carl Mastrangelo
d2cb51bc2e Enable PooledByteBufAllocator to work, event without a cache
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
2017-09-08 10:20:45 +02:00
Norman Maurer
c0396818ca Print out the actual cause when an assertion failure happens during DatagramUnicastTest.testSimpleSendWithConnect
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.
2017-09-07 16:37:46 +02:00
Norman Maurer
a739d89792 Not log notify failure for DelegatingChannelPromiseNotifier when promise is VoidChannelPromise
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.
2017-09-07 08:58:39 +02:00
Nikolay Fedorovskikh
e500755086 Remove double comparing of content out of the DefaultHttp2GoAwayFrame.equals()
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.
2017-09-07 08:30:43 +02:00
Norman Maurer
870b5f5e4b Not add inboundStreamHandler for outbound streams created by Http2MultiplexCodec.
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]
2017-09-06 08:37:29 +02:00
Norman Maurer
5c572f0f63 Ensure the tests complete on java7 and java9 as well.
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.
2017-09-04 20:20:04 +02:00
Norman Maurer
0fffc844d6 Only load native transport if running architecture match the compiled library architecture.
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].
2017-09-04 13:34:55 +02:00
Norman Maurer
379ac890f4 Fix reference count issue when using Http2FrameCodec / Http2MultiplexCodec with HttpServerUpgradeHandler
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].
2017-09-04 13:30:18 +02:00
Norman Maurer
bca35b0449 Allow to construct UnpooledByteBufAllocator that explictly always use sun.misc.Cleaner
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.
2017-08-31 12:57:09 +02:00
Scott Mitchell
9bd6d8129e HttpObjectEncoder buffer size estimation
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.
2017-08-31 01:40:53 -07:00
Carl Mastrangelo
7528e5a11e Use threadsafe setter on Atomic Updaters
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
2017-08-31 10:14:40 +02:00
amizurov
cc336ef2f8 Fix StompSubframeDecoder.readHeaders produce not any notification when parsed line that contains multiple colon
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]
2017-08-29 22:06:45 +02:00
Carl Mastrangelo
c891c9c13f Include more detail why Unsafe is not available
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
2017-08-29 22:02:06 +02:00
Norman Maurer
f5bea11ee4 DefaultSocks5CommandRequest incorrectly rejects SOCKS5 commands with dstPort=0
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].
2017-08-29 15:03:09 +02:00
Stephane Landelle
d9d0e633dc Fix ServerCookieEncoder javadoc, close #7115
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
2017-08-28 20:21:57 +02:00
Francesco Nigro
6780183a89 Makes LengthFieldBasedFrameDecoder::decode inlineable
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.
2017-08-28 09:08:45 +02:00
Paul Gross
0b0de76adc Fix typo in error message
Motivation:

Fix typo in error message.

Modification:

could not fine -> could not find
2017-08-28 09:05:08 +02:00
Scott Mitchell
89ecb4b4a4 AutoClose behavior may infinite loop
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.
2017-08-25 21:01:41 -07:00
Norman Maurer
b967805f32 [maven-release-plugin] prepare for next development iteration 2017-08-24 15:38:22 +02:00
Norman Maurer
da8e010a42 [maven-release-plugin] prepare release netty-4.1.15.Final 2017-08-24 15:37:59 +02:00
Norman Maurer
dd9ad15b12 Add Unit test for [#7143] 2017-08-24 13:29:38 +02:00
Scott Mitchell
c80fdc8241 AbstractChannel should call doClose even after shutdownOutput
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.
2017-08-24 12:30:34 +02:00
Norman Maurer
1065e0f26e Support JDK9-native ALPN
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.
2017-08-24 08:16:51 +02:00
Scott Mitchell
c93e58c453 OpenSsl should use _ instead of -
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.
2017-08-23 22:40:47 -07:00
Norman Maurer
6e859469ca Deprecate ApplicationProtocolNegotiator and its implementation as people should use ApplicationProtocolConfig
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.
2017-08-23 20:18:58 +02:00
Norman Maurer
7290cbc48a More bullet-proof way of detecting if ipv6 is supported or not when using native transport
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].
2017-08-23 13:46:09 +02:00
Derek Perez
b18a201d02 various errorprone fixes.
Motivation:

Continuing to make netty happy when compiling through errorprone.

Modification:

Mostly comments, some minor switch statement changes.

Result:

No more compiler errors!
2017-08-23 12:49:58 +02:00
Scott Mitchell
cbce95eae1 SslHandlerTest#testCompositeBufSizeEstimationGuaranteesSynchronousWrite print SslProvider on failure
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.
2017-08-22 22:20:54 -07:00
Scott Mitchell
566566db3a Decouple DnsCache and DnsCacheEntry
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.
2017-08-21 11:15:27 -07:00
Norman Maurer
2beb5fc8ee DelegatingSslContext should also be able to configure the SslHandler
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.
2017-08-21 20:13:48 +02:00
nitsanw
5b4d2d9807 Update JCTools version, fixes #7117
Motivation:

Remove Unsafe dependency for Atomic queues in JCTools, resolved in version 2.1.0

Modification:

Change pom JCTools version

Result:

Fixes #7117
2017-08-21 19:22:31 +02:00
Carl Mastrangelo
7f1051b6ca Include JNIEXPORT on exported symbols
Motivation:
As noticed in https://stackoverflow.com/questions/45700277/
compilation can fail if the definition of a method doesn't
match the declaration.  It's easy enough to add this in, and make
it easy to compile.

Modifications:
Add JNIEXPORT to the entry points.

* On Windows this adds: `__declspec(dllexport)`
* On Mac this adds: `__attribute__((visibility("default")))`
* On Linux (GCC 4.2+) this adds: ` __attribute__((visibility("default")))`
* On other it doesn't add anything.

Result:
Easier compilation
2017-08-18 17:34:48 -07:00
Scott Mitchell
fe2dd973e9 Unify KQueue and Epoll wait timeout approach
Motivation:
KQueueEventLoop and EpollEventLoop implement different approaches to applying a timeout of their respective poll calls. Epoll attempts to ensure the desired timeout is satisfied at the java layer and at the JNI layer, but it should be sufficient to account for spurious wakups at the JNI layer. Epoll timeout granularity is also limited to milliseconds which may be too large for some latency sensitive applications.

Modifications:
- Make EpollEventLoop wait method look like KQueueEventLoop
- Epoll should support a finer timeout granularity via timerfd_create. We can hide most of these details behind the epollWait0 JNI call to avoid crossing additional JNI boundaries.

Result:
More consistent timeout approach between KQueue and Epoll.
2017-08-18 13:09:02 -07:00
Scott Mitchell
1d7c3fb7ee KQueue detect peer close without EVFILT_READ
Motivation:
The EPOLL transport uses EPOLLRDHUP to detect when the peer closes the write side of the socket. Currently KQueue is not able to mimic this behavior and the only way to detect if the peer has closed is to read. It may not always be appropriate to read for backpressure and other reasons at the application level.

Modifications:
- Support EVFILT_SOCK filter which provides notification when the peer closes the socket

Result:
KQueue transport has more consistent behavior with Epoll transport for detecting peer closure.
2017-08-18 11:00:18 -07:00
Nikolay Fedorovskikh
ba27456653 Use the index-based AsciiString constructor instead of substring()
Motivation:
The construction `new AsciiString(string.substring(...))` can be replaced with the `new AsciiString(string, start, length)` to avoid extra allocation.

Modifications:
Apply the described replacement in `HttpConversionUtil#setHttp2Authority`.

Result:
Less allocations.
2017-08-18 09:48:05 +02:00
Nikolay Fedorovskikh
0234878c4b Fix the commands cache and hashCode() in SmtpCommand
Motivation:
- A `hashCode` of the SmtpCommand is recalculated on each call of `hashCode()`. Cached hash code value can be just replaced with call of `name.hashCode()`.
- The commands cache don't work for strings: `SmtpCommand.valueOf("HELO")` returns a new instance.
- Field `contentExpected` is redundant and can be replaced with `equals(DATA)`.

Modifications:
- Use the `name.hashCode()` as hash code result.
- Fix a command cache: use strings as map keys.
- Replace field `contentExpected` to using `this.equals(DATA)`.
- Add unit tests.

Result:
More correct and clean code.
2017-08-18 09:46:53 +02:00
Carl Mastrangelo
03d89c2222 Use Constructor for reflective class instantiation.
Motivation:
Calling `newInstance()` on a Class object can bypass compile time
checked Exception propagation.  This is noted in Java Puzzlers,
as well as in ErrorProne:
http://errorprone.info/bugpattern/ClassNewInstance

Modifications:
Use the niladic constructor to create a new instance.

Result:
Compile time safety for checked exceptions
2017-08-18 09:42:36 +02:00
Carl Mastrangelo
e4af881bdb Do not define JNI_OnLoad when not dynamic
Motivation:
Due to an oversight (by myself), linking two JNI modules with
duplicate symbols fails in linking.  This only seems to happen
some of the time (the behavior seems to be different between GCC
and Clang toolchains).  For instance, including both netty tcnative
and netty epoll fails to link because of duplicate JNI_OnLoad
symobols.

Modification:
Do not define the JNI_OnLoad and JNI_OnUnload symbols when
compiling for static linkage, as indicated by the NETTY_BUILD_STATIC
preprocessor define.  They are never directly called when
statically linked.

Result:
Able to statically compile epoll and tcnative code into a single
binary.
2017-08-18 09:20:58 +02:00
Norman Maurer
123e07ca80 Revert "Only call ctx.fireChannelReadComplete() if ByteToMessageDecoder decoded at least one message."
This reverts commit d63bb4811e as this not covered correctly all cases and so could lead to missing fireChannelReadComplete() calls. We will re-evalute d63bb4811e and resbumit a pr once we are sure all is handled correctly
2017-08-18 09:06:37 +02:00
Norman Maurer
5de38051c9 Update netty-tcnative native library names to use underscores.
Motivation:

We recently changed netty-tcnative to use underscores in its native library names.

Modifications:

Update code to use underscores when loading native library.

Result:

More consistent code.
2017-08-17 10:08:32 +02:00
Aron Wieck
da86b85a28 Make NativeLibraryLoader check java.library.path first
Motivation:

On restricted systems (e.g. grsecurity), it might not be possible to write a .so on disk and load it afterwards. On those system Netty should check java.library.path for libraries to load.

Modifications:

Changed NativeLibraryLoader.java to first try to load libs from java.library.path before exporting the .so to disk.

Result:

Libraries load fine on restricted systems.
2017-08-16 14:27:50 -07:00
Scott Mitchell
053e6184f2 Increase visibility for SslHandlerTest#testCompositeBufSizeEstimationGuaranteesSynchronousWrite
Motivation:
SslHandlerTest#testCompositeBufSizeEstimationGuaranteesSynchronousWrite has been observed to fail on CI servers, but it is not clear why.

Modifications:
- Add more visibility into what the state was and what the condition that caused the failure was.

Result:
More visibility when the test fails.
2017-08-16 09:53:00 -07:00
Norman Maurer
5d9a5d3e8d Revert SslEngineWrapperFactory api breakage introduced by 4448b8f42f.
Motivation:

Commit 4448b8f42f introduced some API breakage which we need to revert before we release.

Modifications:

- Introduce an AllocatorAwareSslEngineWrapperFactory which expose an extra method that takes a ByteBufAllocator as well.
- Revert API changes to SslEngineWrapperFactory.

Result:

API breakage reverted.
2017-08-16 08:33:46 +02:00
Derek Perez
3469004432 Adding explicit comment about case statement
Motivation:

When compiling this code and running it through errorprone[1], this message appears:
```
StringUtil.java:493: error: [FallThrough] Switch case may fall through; add a `// fall through` comment if it was deliberate
                    case LINE_FEED:
                    ^
    (see http://errorprone.info/bugpattern/FallThrough)
```
By adding that comment, it silences the error and also makes clear the intention of that statement.

[1]http://errorprone.info/index

Modification:

Add simple comment.

Result:

Errorprone is happier with the code.
2017-08-16 07:38:11 +02:00
Norman Maurer
e487db7836 Use the ByteBufAllocator when copy a ReadOnlyByteBufferBuf and so also be able to release it without the GC when the Cleaner is present.
Motivation:

In ReadOnlyByteBufferBuf.copy(...) we just allocated a ByteBuffer directly and wrapped it. This way it was not possible for us to free the direct memory that was used by the copy without the GC.

Modifications:

- Ensure we use the allocator when create the copy and so be able to release direct memory in a timely manner
- Add unit test
- Depending on if the to be copied buffer is direct or heap based we also allocate the same type on copy.

Result:

Fixes [#7103].
2017-08-16 07:33:10 +02:00
Norman Maurer
bbcab32874 Ensure netty builds with java9 (build 9+181)
Motivation:

To be able to build with latest java9 release we need to adjust commons-lang version and maven-enforcer-plugin.

Modifications:

- Use commons-lang 2.6.0
- Use maven-enforcer-plugin 3.0.0.M1 when building with java9

Result:

Netty builds again with latest java9 release
2017-08-15 20:31:10 +02:00
Nikolay Fedorovskikh
0774c91456 Support the little endian floats and doubles by ByteBuf
Motivation:
`ByteBuf` does not have the little endian variant of float/double access methods.

Modifications:
Add support for little endian floats and doubles into `ByteBuf`.

Result:
`ByteBuf` has get/read/set/writeFloatLE() and get/read/set/writeDoubleLE() methods. Fixes [#6576].
2017-08-15 06:24:28 +02:00
Nikolay Fedorovskikh
4875a2aad4 Immediate caching the strings wrapped to AsciiString
Motivation:
The `AsciiString#toString` method calculate string value and cache it into field. If an `AsciiString` created from the `String` value, we can avoid rebuilding strings if we cache them immediately when creating `AsciiString`. It would be useful for constants strings, which already stored in the JVMs string table, or in cases where an unavoidable `#toString `method call is assumed.

Modifications:
- Add new static method `AsciiString#cache(String)` which save string value into cache field.
- Apply a "benign" data race in the `#hashCode` and `#toString` methods.

Result:
Less memory usage in some `AsciiString` use cases.
2017-08-15 06:22:14 +02:00
Norman Maurer
19dcb15062 Use underscore in native library names for consistency.
Motivation:

At the moment we try to load the library using multiple names which includes names using - but also _ . We should just use _ all the time.

Modifications:

Replace - with _

Result:

Fixes [#7069]
2017-08-15 06:02:00 +02:00
Nikolay Fedorovskikh
e249b453a0 Make configurable the initial and max size of InternalThreadLocalMap#stringBuilder
Motivation:
In some cases of using an `InternalThreadLocalMap#stringBuilder`, the `StringBuilder`s size can often exceed the exist limit (1024 bytes). This can lead to permanent memory reallocation.

Modifications:
Add custom properties for the initial capacity and maximum size (after which the `StringBuilder`s capacity will be reduced to the initial capacity).

Result:
An `InternalThreadLocalMap#stringBuilder`s initial and max size is configurable. Fixes [#7092].
2017-08-14 13:27:58 -07:00