Commit Graph

8360 Commits

Author SHA1 Message Date
Scott Mitchell
8cb5d0fa8c SocketGatheringWriteTest increase timeouts
Motivation:
EpollDomainSocketGatheringWriteTest. testGatheringWriteBig takes on average about 20-25 seconds on the CI servers, but there is a 30 second timeout being applied which leads to what maybe false positive test failures.

Modifications:
- Increase the test timeout to 120 seconds globally and 60 seconds to wait for all writes per test

Result:
Higher timeout for potentially less false positive test failures.
2017-08-11 00:15:42 -07:00
Norman Maurer
d0c43c9e42 We should prefer heap buffers when using the OIO transport to reduce memory copies.
Motivation:

When using the OIO transport we need to act on byte[] when writing and reading from / to the underyling Socket. So we should ensure we use heap buffers by default to reduce memory copies.

Modifications:

Ensure we prefer heap buffers by default for the OIO transport.

Result:

Possible less memory copies.
2017-08-11 08:48:04 +02:00
Scott Mitchell
0bf614d9e9 SocketGatheringWriteTest improvements
Motivation:
SocketGatherWriteTest has been observed to fail and it has numerous issues which when resolved may help reduce the test failures.

Modifications:
- A volatile counter and a spin/sleep loop is used to trigger test termination. Incrementing a volatile is generally bad practice and can be avoided in this situation. This mechanism can be replaced by a promise. This mechanism should also trigger upon exception or channel inactive.
- The TestHandler maintains an internal buffer, but it is not released. We now only create a buffer on the server side and release it after comparing the expected results.
- The composite buffer creation logic can be simplified, also the existing composite buffer doesn't take into account the buffer's reader index when building buf2.

Result:
Cleaner test.
2017-08-11 07:16:44 +02:00
Michael Andre Pearce
1ba592049c Add null checks before converting to string to avoid NPE.
Motivation:

Fixing an introduced NPE by #6817

Modification:

Add null checks.

Result:

Fixes #7076 .
2017-08-11 07:14:39 +02:00
Scott Mitchell
ce4b4da583 SocketStringEchoTest improvements
Motivation:
SocketStringEchoTest has been observed to fail and it has numerous issues which when resolved may help reduce the test failures.

Modifications:
- A volatile counter and a spin/sleep loop is used to trigger test termination. Incrementing a volatile is generally bad practice and can be avoided in this situation. This mechanism can be replaced by a promise. This mechanism should also trigger upon exception or channel inactive.
- Asserts are done in the Netty threads. Although these should result in a exceptionCaught the test may not observe these failures because it is spinning waiting for the count to reach the desired value.

Result:
Cleaner test.
2017-08-11 07:12:35 +02:00
Norman Maurer
85f5d6bf05 Ensure we null out the previous set InetAddress on java.net.DatagramPacket when using OioDatagramChannel.
Motivation:

We need to ensure we always null out (or set) the address on the java.net.DatagramPacket when doing read or write operation as the same instance is used across different calls.

Modifications:

Null out the address if needed.

Result:

Ensure the correct remote address is used when connect / disconnect between calls and also mix these with calls that directly specify the remote address for adatagram packets.
2017-08-09 07:31:59 +02:00
Norman Maurer
b30c4f899f Remove debug cruft from e218759c0c 2017-08-08 17:01:31 +02:00
Norman Maurer
e218759c0c Fix regression in detecting macOS/osx platform introduced by bdb0a39c8a 2017-08-08 10:39:22 +02:00
Norman Maurer
f8b495b2f2 Correctly support SO_TIMEOUT for OioDatagramChannel
Motivation:

We need to support SO_TIMEOUT for the OioDatagramChannel but we miss this atm as we not have special handling for it in the DatagramChannelConfig impl that we use. Because of this the following log lines showed up when running the testsuite:

20:31:26.299 [main] WARN  io.netty.bootstrap.Bootstrap - Unknown channel option 'SO_TIMEOUT' for channel '[id: 0x7cb9183c]'

Modifications:

- Add OioDatagramChannelConfig and impl
- Correctly set SO_TIMEOUT in testsuite

Result:

Support SO_TIMEOUT for OioDatagramChannel and so faster execution of datagram related tests in the testsuite
2017-08-08 09:08:33 +02:00
Norman Maurer
bdb0a39c8a Remove code-duplication from NativeLibraryLoader
Motivation:

NativeLibraryLoader has some code-duplication that can be removed.

Modifications:

Remove duplicated code and just use provided methods of PlatformDependent.

Result:

Less code duplication, fixes [#3756].
2017-08-08 09:06:41 +02:00
Violeta Georgieva
db4781282f Handle partially decoded elements while streaming Json array
Motivation:

'insideString' and 'openBraces' need a proper handling when streaming
Json array over multiple writes and an element decoding was started but
not completed.
Related to #6969

Modifications:

If the idx is reset:
- 'insideString' has to be reset to 'false' in order to indicate that
  array element will be decoded from the beginning
- 'openBraces' has to be reset to '1' to indicate that Json array
  decoding is in progress.

Result:
Json array is properly decoded when in streaming mode
2017-08-08 08:48:01 +02:00
Norman Maurer
348745608f Fix incorrect javadocs in Http2RemoteFlowController
Motivation:

The javadocs of Http2RemoteFlowController.isWritable(...) are incorrect.

Modifications:

Update javadocs to reflect reality.

Result:

Correct javadocs.
2017-08-08 07:47:18 +02:00
Scott Mitchell
0f8ecdcad5 Http2FrameLogger frame labels incorrect
Motivation:
The labels identifying the frame types in Http2FrameLogger are not always correct.

Modification:
- Correct the string labels to indicate the right frame type in Http2FrameLogger

Result:
Logs are more correct.
2017-08-07 10:24:17 -07:00
Norman Maurer
918fde82f8 Add testcases to prove HttpResponseEncoder correctly handles empty content
Motivation:

Issue #6695 states that there is an issue when writing empty content via HttpResponseEncoder.

Modifications:

Add two test-cases.

Result:

Verified that all works as expected.
2017-08-07 07:25:33 +02:00
Norman Maurer
bd029503ae asm 6.0_BETA was released so we should use it when building on java9
Motivation:

We used asm 6.0_ALPHA when building on java9 as the latest stable release not works with java9. asm 6.0_BETA was just released so we should update.

Modifications:

Upgrade asm version

Result:

Not use ALPHA release anymore
2017-08-05 08:16:50 +02:00
Scott Mitchell
237a4da1b7 Shutting down the outbound side of the channel should not accept future writes
Motivation:
Implementations of DuplexChannel delegate the shutdownOutput to the underlying transport, but do not take any action on the ChannelOutboundBuffer. In the event of a write failure due to the underlying transport failing and application may attempt to shutdown the output and allow the read side the transport to finish and detect the close. However this may result in an issue where writes are failed, this generates a writability change, we continue to write more data, and this may lead to another writability change, and this loop may continue. Shutting down the output should fail all pending writes and not allow any future writes to avoid this scenario.

Modifications:
- Implementations of DuplexChannel should null out the ChannelOutboundBuffer and fail all pending writes

Result:
More controlled sequencing for shutting down the output side of a channel.
2017-08-04 10:59:57 -07:00
Norman Maurer
d56f403c69 First call channelReadComplete(...) before flush(...) for better performance
Motivation:

In Http2ConnectionHandler we call flush(...) in channelReadComplete(...) to ensure we update the flow-controller and write stuff to the remote peer. We should better flip the order and so may be able to pick up more bytes.

Modifications:

Change order of calls.

Result:

Better performance
2017-08-04 11:55:35 +02:00
Norman Maurer
d63bb4811e Only call ctx.fireChannelReadComplete() if ByteToMessageDecoder decoded at least one message.
Motivation:

Its wasteful and also confusing that channelReadComplete() is called even if there was no message forwarded to the next handler.

Modifications:

- Only call ctx.fireChannelReadComplete() if at least one message was decoded
- Add unit test

Result:

Less confusing behavior. Fixes [#4312].
2017-08-04 10:54:56 +02:00
Scott Mitchell
a75ac747f0 Remove io.netty.packagePrefix system property
Motivation:
Now that the NativeLibraryLoader implicitly detects the shaded package prefix we no longer need the io.netty.packagePrefix system property.

Modifications:
- Remove io.netty.packagePrefix processing from NativeLibraryLoader

Result:
Code is cleaner.
2017-08-04 10:53:29 +02:00
Norman Maurer
32f497760f Ensure no java.lang.UnsupportedClassVersionError are thrown if running on Java7 and try to check if conscrypt is available.
Motivation:

We need to ensure we not try to load any conscrypt classes directly (which means without using reflection) in the same class that is used to check if conscrypt is available. This is needed as otherwise we will have the following problem when try to use netty on java7:

java.lang.UnsupportedClassVersionError: org/conscrypt/BufferAllocator : Unsupported major.minor version 52.0
	at io.netty.handler.ssl.ConscryptJdkSslEngineInteropTest.checkConscrypt(ConscryptJdkSslEngineInteropTest.java:49)

This regression was introduced by 4448b8f42f and detected on the CI when using:

mvn clean package -DtestJavaHome=$JAVA7_HOME

Modifications:

Move the detection code in an extra class and use it.

Result:

Works correctly also when using Java7.
2017-08-04 10:48:50 +02:00
Norman Maurer
4bb89dcc54 Correctly handle connect/disconnect in EpollDatagramChannel / KQueueDatagramChannel
Motivation:

We did not correctly handle connect() and disconnect() in EpollDatagramChannel / KQueueDatagramChannel and so the behavior was different compared to NioDatagramChannel.

Modifications:

- Correct implement connect and disconnect methods
- Share connect and related code
- Add tests

Result:

EpollDatagramChannel / KQueueDatagramChannel also supports correctly connect() and disconnect() methods.
2017-08-04 09:22:53 +02:00
Nathan Mittler
4448b8f42f Upgrading to Conscrypt 1.0.0.RC9. (#7044)
Motivation:

Starting with 1.0.0.RC9, conscrypt supports a buffer allocator.

Modifications:

- Updated the creation process for the engine to pass through the
ByteBufAllocator.
- Wrap a ByteBufAllocator with an adapter for conscrypt.
- Added a property to optionally control whether conscrypt uses
Netty's buffer allocator.

Result:

Netty+conscrypt will support using Netty's ByteBufAllocator.
2017-08-03 14:21:32 -07:00
Norman Maurer
52f384b37f [maven-release-plugin] prepare for next development iteration 2017-08-02 12:55:10 +00:00
Norman Maurer
8cc1071881 [maven-release-plugin] prepare release netty-4.1.14.Final 2017-08-02 12:54:51 +00:00
chhsiao90
8320a45c15 Configures HTTP2 pipeline with more proper way
Motivation:

When we use pipeline.replace and we still had ongoing inbound, then
there will be some problem that inbound message would go to wrong
handlers. So we add handler first, and remove self after add, so that
the next handler will be the correct one.

Modifications:

Uses remove after addAfter instead of replace.

Result:

Fixed #6881
2017-08-02 06:58:55 +02:00
Nikolay Fedorovskikh
6ab9c177ac Fix hash function and hash table size in Snappy
Motivation:

1. Hash function in the Snappy encoding is wrong probably: used '+' instead of '*'. See the reference implementation [1].
2. Size of the hash table is calculated, but not applied.

Modifications:

1. Fix hash function: replace addition by multiplication.
2. Allocate hash table with calculated size.
3. Use an `Integer.numberOfLeadingZeros` trick for calculate log2.
4. Release buffers in tests.

Result:

1. Better compression. In the test `encodeAndDecodeLongTextUsesCopy` now compressed size is 175 instead of 180 before this change.
2. No redundant allocations for hash table.
3. A bit faster the calc of shift (less an expensive math operations).

[1] 513df5fb5a/snappy.cc (L67)
2017-08-01 07:08:54 +02:00
Norman Maurer
2988fb8eeb Ensure Http2FrameCodec uses Http2Settings.defaultSettings()
Motivation:

Http2FrameCodec should use Http2Settings.defaultSettings() when no Http2Settings were specified by the user.

Modifications:

Replace new Http2Settings() with Http2Settings.defaultSettings()

Result:

Use correct Http2Settings by default when using Http2FrameCodec in all cases.
2017-08-01 07:07:05 +02:00
Nikolay Fedorovskikh
068e64dbcf Fix a potential NPE 2017-07-31 20:39:20 +02:00
Alberto K
21b7ab1f25 Allows IP_TRANSPARENT to be set on a redirecting socket
Motivation:

IP_TRANSPARENT support is not complete, the option can currently only be set on EpollServerSocket. Setting the option on an EpollSocket is also requires so as to be able to bind a socket to a non-local address as described in ip(7)
http://man7.org/linux/man-pages/man7/ip.7.html

"TProxy redirection with the iptables TPROXY target also
requires that this option be set on the redirected socket."

Modifications:

Added IP_TRANSPARENT socket option to EpollSocketChannelConfig

Result:

A redirecting socket can be created with a non-local IP address as required for TPROXY
2017-07-30 19:51:55 +02:00
Norman Maurer
580ac8cd41 Only flush on channelReadComplete(...) in http2 hello world examples.
Motivation:

In our http1 hello world example we only flush on channelReadComplete(...) to make better use of gathering writes. We should do the same in http2.

Modifications:

Only flush in channelReadComplete(...)

Result:

Better performance and more consistent examples.
2017-07-30 07:18:49 +02:00
Norman Maurer
8adb30bbe2 Correctly run all pending tasks for EmbeddedChannel when the Channel was closed.
Motivation:

When a user called ctx.close() and used the EmbeddedChannel we did not correctly run all pending tasks which means channelInactive was never called.

Modifications:

Ensure we run all pending tasks after all operations that may change the Channel state and are part of the Channel.Unsafe impl.

Result:

Fixes [#6894].
2017-07-30 06:57:18 +02:00
Norman Maurer
d9d3d65716 Add comment why the ResourceLeak creation is happening as last in the constructor. Followup of c5b5d36360 2017-07-30 06:55:22 +02:00
Norman Maurer
c5b5d36360 Fix false-positive leak detection report when ReferenceCountedOpenSslEngine constructor throws.
Motivation:

We need to ensure we only create the ResourceLeak when the constructor not throws.

Modifications:

Ensure ResourceLeakDetector.track(...) is only called if the constructor of ReferenceCoundedOpenSslEngine not throws.

Result:

No more false-positves.
2017-07-29 22:01:56 +02:00
Carl Mastrangelo
60250f3795 Make DelegatingChannelPromiseNotifier use Vararg overload
Motivation:
ErrorProne complains that the array override doesn't match the
vararg super call.  See http://errorprone.info/bugpattern/Overrides

Additionally, almost every other Future uses the vararg form, so
it would be stylistically consistent to keep it that way.

Modifications:
Use vararg override.

Result:
Cleaner, less naggy code.
2017-07-28 07:29:43 +02:00
Norman Maurer
339131c660 DefaultChannelPipeline.estimatorHandle needs to be volatile
Motivation:

DefaultChannelPipeline.estimatorHandle needs to be volatile as its accessed from different threads.

Modifications:

Make DefaultChannelPipeline.estimatorHandle volatile and correctly init it via CAS

Result:

No more race.
2017-07-27 06:57:22 +02:00
Vladimir Gordiychuk
fe8ecea366 Http2FrameLogger avoid hex dump of the ByteBufs when log disabled
Motivation:

Currentry logger create hex dump even if log write will not apply.
It's unecessary GC overhead.

Modifications:

Restore optimization from #3492

Result:

Fixes #7025
2017-07-26 21:21:04 +02:00
Spencer Fang
732b145842 Http2ConnectionHandler: allow graceful shutdown to wait forever
Motivation:

There should be a way to allow graceful shutdown to wait for all open streams to close without a timeout. Using gracefulShutdownTimeoutMillis with a large value is a bit of a hack, and has a gotcha that sufficiently large values will overflow the long, resulting in a ClosingChannelFutureListener that executes immediately.

Modification:

Allow to use gracefulShutdownTimeoutMillis(-1) to express waiting until all streams are closed.

Result:

We can now shutdown the connection without a forced timeout.
2017-07-26 20:40:24 +02:00
Norman Maurer
529025d9d5 Allow to use oldest Channel out of the Simple / FixedChannelPool on acquire
Motivation:

We previously used pollLast() to retrieve a Channel from the queue that backs SimpleChannelPool. This could lead to the problem that some Channels are very unfrequently used and so when these are used the connection was already be closed and so could not be reused.

Modifications:

Allow to configure if the last recent used Channel should be used or the "oldest".

Result:

More flexible usage of ChannelPools
2017-07-26 20:37:19 +02:00
Norman Maurer
359beff56f Choose ipv4 or ipv6 google dns servers as default fallback based on the settings for this system / jvm
Motivation:

We should not use ipv4 google dns servers if the app is configured to run ipv6.

Modifications:

Use either ipv4 or ipv6 dns servers depending on the system config.

Result:

More correct behaviour
2017-07-26 20:33:52 +02:00
Norman Maurer
efb2d141c1 Correctly handle unsigned int values returned from TCP_INFO
Motivation:

We used an int[] to store all values that are returned in the struct for TCP_INFO which is not good enough as it uses usigned int values.

Modifications:

- Change int[] to long[] and correctly cast values.

Result:

No more truncated values.
2017-07-25 16:18:52 +02:00
Norman Maurer
34fdc7a33e Skip invalid hostnames when construct default dns servers to use.
Motivation:

When the hostname portion can not be extracted we should just skip the server as otherwise we will produce and exception when trying to create the InetSocketAddress.

This was happing when trying to run the test-suite on a system and using java7:

java.lang.IllegalArgumentException: hostname can't be null
	at java.net.InetSocketAddress.checkHost(InetSocketAddress.java:149)
	at java.net.InetSocketAddress.<init>(InetSocketAddress.java:216)
	at io.netty.util.internal.SocketUtils$10.run(SocketUtils.java:171)
	at io.netty.util.internal.SocketUtils$10.run(SocketUtils.java:168)
	at java.security.AccessController.doPrivileged(Native Method)
	at io.netty.util.internal.SocketUtils.socketAddress(SocketUtils.java:168)
	at io.netty.resolver.dns.DefaultDnsServerAddressStreamProvider.<clinit>(DefaultDnsServerAddressStreamProvider.java:74)
	at io.netty.resolver.dns.DnsServerAddressesTest.testDefaultAddresses(DnsServerAddressesTest.java:39)

Modifications:

Skip if hostname can not be extracted.

Result:

No more java.lang.ExceptionInInitializerError.
2017-07-25 08:43:53 +02:00
Norman Maurer
486f962252 Respect DNS port that is specified via JNDI
Motivation:

JNDI allows to specify an port so we should respect it.

Modifications:

Use the specified port and if none is specifed use 53.

Result:

Correct handling of JNDI configured DNS.
2017-07-25 08:26:59 +02:00
Carl Mastrangelo
d8f4547f5c Unify {Epoll,KQueue}EventLoopGroup initialization.
Motivation:
`Epoll.ensureAvailability()` is called multiple times, once in
static initialization and in a couple of the constructors.  This is
redundant and confusing to read.

Modifications:
Move `Epoll.ensureAvailability()` call into an instance initializer
and remove all other references.  This ensures that every EELG
checks availability, while still delaying the check until
construction.  This pattern is used when there are multiple ctors,
as in this class.

Result:
Easier to read code.
2017-07-24 20:14:54 +02:00
Scott Mitchell
452fd36240 ByteBufs which are not resizable should not throw in ensureWritable(int,boolean)
Motivation:
ByteBuf#ensureWritable(int,boolean) returns an int indicating the status of the resize operation. For buffers that are unmodifiable or cannot be resized this method shouldn't throw but just return 1.
ByteBuf#ensureWriteable(int) should throw unmodifiable buffers.

Modifications:
- ReadOnlyByteBuf should be updated as described above.
- Add a unit test to SslHandler which verifies the read only buffer can be tolerated in the aggregation algorithm.

Result:
Fixes https://github.com/netty/netty/issues/7002.
2017-07-22 08:44:48 -07:00
Eric Anderson
2fbce6d470 Delete temporary self-signed certs in SSLEngineTest-based tests
Motivation:

Lots of usages of SelfSignedCertificates were not deleting the certs at
the end of the test. This includes setupHandlers() which is used by
extending classes. Although these files will be deleted at JVM exit and
deleting them early does not free the JVM from trying to delete them at
shutdown, it's good practice to delete eagerly and since users sometimes
use tests as a form of documentation, it'd be good for them to see the
explicit deletes.

Modifications:

Add missing delete() calls to ½ of the SelfSignedCertificates-using
tests.

Result:

Tests that more clearly communicates which resources are created and
may accumulate without early delete.
2017-07-22 08:13:57 +02:00
Eric Anderson
8a25c35939 Filter user-provided ciphers using RFC cipher names
Motivation:

Previously filterCipherSuites was being passed the OpenSSL-formatted
cipher names. Commit 43ae974 introduced a regression as it swapped to the
RFC/JDK format, except that user-provided ciphers were not converted and
remained in the OpenSSL format.

This mis-match would cause all user-provided to be thrown away, leading
to failure trying to set zero ciphers:
Exception in thread "main" javax.net.ssl.SSLException: failed to set cipher suite: []
	at io.netty.handler.ssl.ReferenceCountedOpenSslContext.<init>(ReferenceCountedOpenSslContext.java:299)
	at io.netty.handler.ssl.OpenSslContext.<init>(OpenSslContext.java:43)
	at io.netty.handler.ssl.OpenSslServerContext.<init>(OpenSslServerContext.java:347)
	at io.netty.handler.ssl.OpenSslServerContext.<init>(OpenSslServerContext.java:335)
	at io.netty.handler.ssl.SslContext.newServerContextInternal(SslContext.java:421)
	at io.netty.handler.ssl.SslContextBuilder.build(SslContextBuilder.java:441)
Caused by: java.lang.Exception: Unable to configure permitted SSL ciphers (error:100000b1:SSL routines:OPENSSL_internal:NO_CIPHER_MATCH)
	at io.netty.internal.tcnative.SSLContext.setCipherSuite(Native Method)
	at io.netty.handler.ssl.ReferenceCountedOpenSslContext.<init>(ReferenceCountedOpenSslContext.java:295)
	... 7 more

Modifications:

Remove the reformatting of user-provided ciphers, as they are already in
the RFC/JDK format.

Result:

No regression, and the internals stay sane using the RFC/JDK format.
2017-07-21 19:20:07 -07:00
Norman Maurer
d141ba11bf Fix flacky multipart test introduced by 08748344d8.
Motivation:

08748344d8 introduced two new tests which did not take into account that the multipart delimiter can be between 2 and 16 bytes long.

Modifications:

Take the multipart delimiter length into account.

Result:

Fixes [#7001]
2017-07-21 14:28:32 +02:00
Norman Maurer
06f64948d5 Add tests to ensure an IllegalReferenceCountException is thrown if set/writeCharSequence is called on a released buffer
Motivation:

We need to ensure we not allow calling set/writeCharsequence on an released ByteBuf.

Modifications:

Add test-cases

Result:

Proves fix of [#6951].
2017-07-21 07:39:32 +02:00
martin vseticka
f897507b09 Handle handshake failure in Websocket Client example
Motivation:

We need to fail the promise if a failure during handshake happens.

Modification:

Correctly fail the promise.

Result:

Correct websocket client example. Fixes [#6998]
2017-07-21 07:36:36 +02:00
Norman Maurer
f23b2fc25d Use 4 spaces and not 2 spaces (cleanup of 3d22b24244) 2017-07-20 10:24:18 +02:00