Commit Graph

1811 Commits

Author SHA1 Message Date
Norman Maurer
bcad9dbf97 Revert "Set readPending to false when ever a read is done"
This reverts commit 413c7c2cd8 as it introduced an regression when edge-triggered mode is used which is true for our native transports by default. With 413c7c2cd8 included it was possible that we set readPending to false by mistake even if we would be interested in read more.
2017-11-06 09:21:42 -08:00
Scott Mitchell
413c7c2cd8 Set readPending to false when ever a read is done
Motivation:
readPending is currently only set to false if data is delivered to the application, however this may result in duplicate events being received from the selector in the event that the socket was closed.

Modifications:
- We should set readPending to false before each read attempt for all
transports besides NIO.
- Based upon the Javadocs it is possible that NIO may have spurious
wakeups [1]. In this case we should be more cautious and only set
readPending to false if data was actually read.

[1] https://docs.oracle.com/javase/7/docs/api/java/nio/channels/SelectionKey.html
That a selection key's ready set indicates that its channel is ready for some operation category is a hint, but not a guarantee, that an operation in such a category may be performed by a thread without causing the thread to block.

Result:
Notification from the selector (or simulated events from kqueue/epoll ET) in the event of socket closure.
Fixes https://github.com/netty/netty/issues/7255
2017-10-25 08:25:54 -07:00
Norman Maurer
55b501d0d4 Correctly update Channel writability when queueing data in SslHandler.
Motivation:

A regression was introduced in 86e653e which had the effect that the writability was not updated for a Channel while queueing data in the SslHandler.

Modifications:

- Factor out code that will increment / decrement pending bytes and use it in AbstractCoalescingBufferQueue and PendingWriteQueue
- Add test-case

Result:

Channel writability changes are triggered again.
2017-10-24 09:13:15 +02:00
Idel Pivnitskiy
558097449c Add missed 'serialVersionUID' field for Serializable classes
Motivation:

Without a 'serialVersionUID' field, any change to a class will make
previously serialized versions unreadable.

Modifications:

Add missed 'serialVersionUID' field for all Serializable
classes.

Result:

Proper deserialization of previously serialized objects.
2017-10-21 14:41:18 +02:00
Sky Ao
47f016bf56 extract self() method to avoid too many unchecked @SuppressWarnings
Motivation:

There are many @SuppressWarnings("unchecked") in the code for the same purpose that we want to do this return:

@SuppressWarnings("unchecked")
public B someMethod() {
    ......
    return (B) this;
}
Modification:

Add a method self() and reuse in all these return lines:

@SuppressWarnings("unchecked")
    private B self() {
        return (B) this;
    }
Result:

Then only one @SuppressWarnings("unchecked") left in the code.
2017-09-27 13:25:16 +02:00
Norman Maurer
625a7426cd [maven-release-plugin] prepare for next development iteration 2017-09-25 06:12:32 +02:00
Norman Maurer
f57d8f00e1 [maven-release-plugin] prepare release netty-4.1.16.Final 2017-09-25 06:12:16 +02:00
Norman Maurer
aa8bdb5d6b Fix assertion error when closing / shutdown native channel and SO_LINGER is set.
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].
2017-09-18 14:46:37 -07:00
Nikolay Fedorovskikh
e404690574 Optimize DefaultChannelId.equals
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.
2017-09-08 10:36:48 +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
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
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
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
Norman Maurer
74f24a5c19 Finish work on http2 child channel implementation and http2 frame api.
Motivation:

Our http2 child channel implementation was not 100 % complete and had a few bugs. Beside this the performance overhead was non-trivial.

Modifications:

There are a lot of modifications, the most important....
  * Http2FrameCodec extends Http2ConnectionHandler and Http2MultiplexCodec extends Http2FrameCodec to reduce performance heads and inter-dependencies on handlers in the pipeline
  * Correctly handle outbound flow control for child channels
  * Support unknow frame types in Http2FrameCodec and Http2MultiplexCodec
  * Use a consistent way how to create Http2ConnectionHandler, Http2FrameCodec and Http2MultiplexCodec (via a builder)
  * Remove Http2Codec and Http2CodecBuilder as the user should just use Http2MultipleCodec and Http2MultiplexCodecBuilder now
  * Smart handling of flushes from child channels to reduce overhead
  * Reduce object allocations
  * child channels always use the same EventLoop as the parent Channel to reduce overhead and simplify implementation.
  * Not extend AbstractChannel for the child channel implementation to reduce overhead in terms of performance and memory usage
  * Remove Http2FrameStream.managedState(...) as the user of the child channel api should just use Channel.attr(...)

Result:

Http2MultiplexCodec (and so child channels) and Http2FrameCodec are more correct, faster and more feature complete.
2017-08-11 12:41:28 +02:00
buchgr
5380c7c3e3 HTTP/2 Child Channel and FrameCodec Feature Parity.
Motivation:

This PR (unfortunately) does 4 things:
1) Add outbound flow control to the Http2MultiplexCodec:
   The HTTP/2 child channel API should interact with HTTP/2 outbound/remote flow control. That is,
   if a H2 stream used up all its flow control window, the corresponding child channel should be
   marked unwritable and a writability-changed event should be fired. Similarly, a unwritable
   child channel should be marked writable and a writability-event should be fired, once a
   WINDOW_UPDATE frame has been received. The changes are (mostly) contained in ChannelOutboundBuffer,
   AbstractHttp2StreamChannel and Http2MultiplexCodec.

2) Introduce a Http2Stream2 object, that is used instead of stream identifiers on stream frames. A
   Http2Stream2 object allows an application to attach state to it, and so a application handler
   no longer needs to maintain stream state (i.e. in a map(id -> state)) himself.

3) Remove stream state events, which are no longer necessary due to the introduction of Http2Stream2.
   Also those stream state events have been found hard and complex to work with, when porting gRPC
   to the Http2FrameCodec.

4) Add support for HTTP/2 frames that have not yet been implemented, like PING and SETTINGS. Also add
   a Http2FrameCodecBuilder that exposes options from the Http2ConnectionHandler API that couldn't else
   be used with the frame codec, like buffering outbound streams, window update ratio, frame logger, etc.

Modifications:

1) A child channel's writability and a H2 stream's outbound flow control window interact, as described
   in the motivation. A channel handler is free to ignore the channel's writability, in which case the
   parent channel is reponsible for buffering writes until a WINDOW_UPDATE is received.

   The connection-level flow control window is ignored for now. That is, a child channel's writability
   is only affected by the stream-level flow control window. So a child channel could be marked writable,
   even though the connection-level flow control window is zero.

2) Modify Http2StreamFrame and the Http2FrameCodec to take a Http2Stream2 object intstead of a primitive
   integer. Introduce a special Http2ChannelDuplexHandler that has newStream() and forEachActiveStream()
   methods. It's recommended for a user to extend from this handler, to use those advanced features.

3) As explained in the documentation, a new inbound stream active can be detected by checking if the
   Http2Stream2.managedState() of a Http2HeadersFrame is null. An outbound stream active can be detected
   by adding a listener to the ChannelPromise of the write of the first Http2HeadersFrame. A stream
   closed event can be listened to by adding a listener to the Http2Stream2.closeFuture().

4) Add a simple Http2FrameCodecBuilder and implement the missing frame types.

Result:

1) The Http2MultiplexCodec supports outbound flow control.
2) The Http2FrameCodec API makes it easy for a user to manage custom stream specific state and to create
   new outbound streams.
3) The Http2FrameCodec API is much cleaner and easier to work with. Hacks like the ChannelCarryingHeadersFrame
   are no longer necessary.
4) The Http2FrameCodec now also supports PING and SETTINGS frames. The Http2FrameCodecBuilder allows the Http2FrameCodec
   to use some of the rich features of the Http2ConnectionHandler API.
2017-08-11 12:41:28 +02: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
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
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
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
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
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
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
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
ef22e65b57 Allow to delay registration when creating a EmbeddedChannel
Motivation:

Some ChannelOptions must be set before the Channel is really registered to have the desired effect.

Modifications:

Add another constructor argument which allows to not register the EmbeddedChannel to its EventLoop until the user calls register().

Result:

More flexible usage of EmbeddedChannel. Also Fixes [#6968].
2017-07-19 19:35:03 +02:00
Norman Maurer
4c14d1198b Add testcase to ensure NioEventLoop.rebuildSelector() works correctly.
Motivation:

We had recently a report that the issue [#6607] is still not fixed.

Modifications:

Add a testcase to prove the issue is fixed.

Result:

More tests.
2017-07-18 07:20:16 +02:00
Scott Mitchell
7cfe416182 Use unbounded queues from JCTools 2.0.2
Motivation:
JCTools 2.0.2 provides an unbounded MPSC linked queue. Before we shaded JCTools we had our own unbounded MPSC linked queue and used it in various places but gave this up because there was no public equivalent available in JCTools at the time.

Modifications:
- Use JCTool's MPSC linked queue when no upper bound is specified

Result:
Fixes https://github.com/netty/netty/issues/5951
2017-07-10 12:32:15 -07:00
Scott Mitchell
86e653e04f SslHandler aggregation of plaintext data on write
Motivation:
Each call to SSL_write may introduce about ~100 bytes of overhead. The OpenSslEngine (based upon OpenSSL) is not able to do gathering writes so this means each wrap operation will incur the ~100 byte overhead. This commit attempts to increase goodput by aggregating the plaintext in chunks of <a href="https://tools.ietf.org/html/rfc5246#section-6.2">2^14</a>. If many small chunks are written this can increase goodput, decrease the amount of calls to SSL_write, and decrease overall encryption operations.

Modifications:
- Introduce SslHandlerCoalescingBufferQueue in SslHandler which will aggregate up to 2^14 chunks of plaintext by default
- Introduce SslHandler#setWrapDataSize to control how much data should be aggregated for each write. Aggregation can be disabled by setting this value to <= 0.

Result:
Better goodput when using SslHandler and the OpenSslEngine.
2017-07-10 12:22:08 -07:00
Norman Maurer
2a376eeb1b [maven-release-plugin] prepare for next development iteration 2017-07-06 13:24:06 +02:00
Norman Maurer
c7f8168324 [maven-release-plugin] prepare release netty-4.1.13.Final 2017-07-06 13:23:51 +02:00
Norman Maurer
91b62da8c1 Remove @deprecation keyword on AbstractUnsafe.ensureOpen(...)
Motivation:

e845670043 marked AbstractUnsafe.ensureOpen(...) as deprecated for no reason.

Modifications:

Remove `@deprecation`

Result:

Remove incorrect annotation
2017-06-29 14:28:55 +02:00
Norman Maurer
b7a5743e8b Return the correct Future from FixedChannelPool.release()
Motivation:

The behaviour of the FixedChannelPool.release was inconsistent with the
SimpleChannelPool implementation, in that given promise is returned.

In the FixedChannelPool implementation a new promise was return and
this meant that the completion of that promise can be different.
Specifically on releasing a channel to a closed pool, the parameter
promise is failed with an IllegalStateException but the returned one
will have been successful (as it was completed by call to super
.release)

Modification:

Return the given promise as the result of FixedChannelPool.release

Result:

Returned promise will reflect the result of the release operation.
2017-06-28 18:57:10 +02:00
Norman Maurer
32b3f58f63 Close channels that are released to a closed FixedChannelPool.
Motivation:

Channels returned to a FixedChannelPool after closing it remain active.

Since channels that where acquired from the pool are not closed during the close operation, they remain open even after releasing the channel back to the pool where they are then in accessible and become in-effect a connection leak.

Modification:

Close the released channel on releasing back to a closed pool.

Result:

Much harder to create a connection leak by closing an active
FixedChannelPool instance.
2017-06-28 18:50:51 +02:00
Norman Maurer
94c0ef3c96 Not fail the promise when a closed Channel is offered back to the ChannelPool
Motivation:

We should not fail the promise when a closed Channel is offereed back to the ChannelPool as we explicit mention that the Channel must always be returned.

Modifications:

- Not fail the promise
- Add test-case

Result:

Fixes [#6831]
2017-06-13 18:50:20 +02:00
Norman Maurer
fd67a2354d [maven-release-plugin] prepare for next development iteration 2017-06-08 21:06:24 +02:00
Norman Maurer
3acd5c68ea [maven-release-plugin] prepare release netty-4.1.12.Final 2017-06-08 21:06:01 +02:00
Norman Maurer
80aa5dcdcc Revert "Not add ChannelHandler to ChannelPipeline once the pipeline was destroyed."
This reverts commit 4aa8002596.
2017-06-08 19:50:17 +02:00
Norman Maurer
4aa8002596 Not add ChannelHandler to ChannelPipeline once the pipeline was destroyed.
Motivation:

ChannelPipeline will happily add a handler to a closed Channel's pipeline and will call handlerAdded(...) but will not call handlerRemoved(...).

Modifications:

Check if pipeline was destroyed and if so not add the handler at all but propergate an exception.

Result:

Fixes [#6768]
2017-05-31 07:37:39 +02:00
Norman Maurer
0db2901f4d [maven-release-plugin] prepare for next development iteration 2017-05-11 16:00:55 +02:00
Norman Maurer
f7a19d330c [maven-release-plugin] prepare release netty-4.1.11.Final 2017-05-11 16:00:16 +02:00
Scott Mitchell
3cc4052963 New native transport for kqueue
Motivation:
We currently don't have a native transport which supports kqueue https://www.freebsd.org/cgi/man.cgi?query=kqueue&sektion=2. This can be useful for BSD systems such as MacOS to take advantage of native features, and provide feature parity with the Linux native transport.

Modifications:
- Make a new transport-native-unix-common module with all the java classes and JNI code for generic unix items. This module will build a static library for each unix platform, and included in the dynamic libraries used for JNI (e.g. transport-native-epoll, and eventually kqueue).
- Make a new transport-native-unix-common-tests module where the tests for the transport-native-unix-common module will live. This is so each unix platform can inherit from these test and ensure they pass.
- Add a new transport-native-kqueue module which uses JNI to directly interact with kqueue

Result:
JNI support for kqueue.
Fixes https://github.com/netty/netty/issues/2448
Fixes https://github.com/netty/netty/issues/4231
2017-05-03 09:53:22 -07:00
Guanpeng Xu
2d38a4453c Remove the comment which is a bit misleading
This fixes #6652.

Rationale

The invocation of initChannel of ChannelInitializer has been moved to as
early as during handlerAdded is invoked in 26aa34853, whereas it was
only invoked during channelRegistered is invoked before that. So the
comment does not describe how handlers are added in normal circumstances
anymore.

However, the code is kept as-is since there might be unusual cases, and
adding ServerBootstrapAcceptor via the event loop is always safe to
enforce the correct order.
2017-05-02 15:36:20 -07:00
Norman Maurer
6915ec3bb9 [maven-release-plugin] prepare for next development iteration 2017-04-29 14:10:00 +02:00
Norman Maurer
f30f242fee [maven-release-plugin] prepare release netty-4.1.10.Final 2017-04-29 14:09:32 +02:00
Jason Tedor
98beb777f8 Enable configuring available processors
Motivation:

In cases when an application is running in a container or is otherwise
constrained to the number of processors that it is using, the JVM
invocation Runtime#availableProcessors will not return the constrained
value but rather the number of processors available to the virtual
machine. Netty uses this number in sizing various resources.
Additionally, some applications will constrain the number of threads
that they are using independenly of the number of processors available
on the system. Thus, applications should have a way to globally
configure the number of processors.

Modifications:

Rather than invoking Runtime#availableProcessors, Netty should rely on a
method that enables configuration when the JVM is started or by the
application. This commit exposes a new class NettyRuntime for enabling
such configuraiton. This value can only be set once. Its default value
is Runtime#availableProcessors so that there is no visible change to
existing applications, but enables configuring either a system property
or configuring during application startup (e.g., based on settings used
to configure the application).

Additionally, we introduce the usage of forbidden-apis to prevent future
uses of Runtime#availableProcessors from creeping. Future work should
enable the bundled signatures and clean up uses of deprecated and
other forbidden methods.

Result:

Netty can be configured to not use the underlying number of processors,
but rather the constrained number of processors.
2017-04-23 10:31:17 +02:00