Commit Graph

1769 Commits

Author SHA1 Message Date
Norman Maurer
89e93968ac Remove usage of own Atomic*FieldUpdater in favor of JDKs
Motivation:

In later Java8 versions our Atomic*FieldUpdater are slower then the JDK implementations so we should not use ours anymore. Even worse the JDK implementations provide for example an optimized version of addAndGet(...) using intrinsics which makes it a lot faster for this use-case.

Modifications:

- Remove methods that return our own Atomic*FieldUpdaters.
- Use the JDK implementations everywhere.

Result:

Faster code.
2016-12-15 08:09:06 +00:00
Scott Mitchell
002c99e751 NIO ServerChannel shouldn't close because of Exception
Motivation:
e102a008b6 changed a conditional where previously the NIO ServerChannel would not be closed in the event of an exception.

Modifications:
- Restore the logic prior to e102a008b6 which does not automatically close ServerChannels for IOExceptions

Result:
NIO ServerChannel doesn't close automatically for an IOException.
2016-12-05 20:51:05 -08:00
Norman Maurer
eed6791f8e Cleanup after commit fc1cdc991e 2016-12-05 12:18:35 +01:00
Norman Maurer
fc1cdc991e [#6095] Remove catching of ConcurrentModificationException as this can not happen.
Motivation:

We should not catch ConcurrentModificationException as this can never happen because things are executed on the EventLoop thread.

Modifications:

Remove try / catch

Result:

Cleaner code.
2016-12-04 18:59:10 +01:00
Derbylock
f6b37a38ba Removed final keyword from FixedChannelPool 2016-11-23 13:37:37 +01:00
Scott Mitchell
a25101dd0b Now that LocalChannel#releaseInboundBuffers is only called from the EventLoop (eb4d317b9d) it should clear readInProgress and drain/release the queue. Otherwise if a read event is pending (doBeginRead) was called we may later call channelRead or channelReadComplete after we have closed the channel.
Modifications:
LocalChannel#releaseInboundBuffers should always clear/release the queue and set readInProgress to false

Result:
LocalChannel queue is more reliably cleaned up.
2016-11-21 11:11:43 -08:00
Scott Mitchell
eb4d317b9d Fix LocalChannel close sequence
Motivation:
LocalChannel attempts to close its peer socket when ever it is closed. However if the channels are on different EventLoops we may attempt to process events for the peer channel on the wrong EventLoop.

Modifications:
- Ensure the close process ensures we are on the correct thread before accessing data

Result:
More correct LocalChannel close code.
2016-11-21 10:33:55 -08:00
Scott Mitchell
a043cf4a98 Catch exceptions from PlatformDependent#getSystemClassLoader
Motivation:
PlatformDependent#getSystemClassLoader may throw a wide variety of exceptions based upon the environment. We should handle all exceptions and continue initializing the slow path if an exception occurs.

Modifications:
- Catch Throwable in cases where PlatformDependent#getSystemClassLoader is used

Result:
Fixes https://github.com/netty/netty/issues/6038
2016-11-19 09:23:25 -08:00
Scott Mitchell
c1932a8537 ByteBuf Input Stream Reference Count Ownership
Motivation:
Netty provides a adaptor from ByteBuf to Java's InputStream interface. The JDK Stream interfaces have an explicit lifetime because they implement the Closable interface. This lifetime may be differnt than the ByteBuf which is wrapped, and controlled by the interface which accepts the JDK Stream. However Netty's ByteBufInputStream currently does not take reference count ownership of the underlying ByteBuf. There may be no way for existing classes which only accept the InputStream interface to communicate when they are done with the stream, other than calling close(). This means that when the stream is closed it may be appropriate to release the underlying ByteBuf, as the ownership of the underlying ByteBuf resource may be transferred to the Java Stream.

Motivation:
- ByteBufInputStream.close() supports taking reference count ownership of the underyling ByteBuf

Result:
ByteBufInputStream can assume reference count ownership so the underlying ByteBuf can be cleaned up when the stream is closed.
2016-11-14 16:29:55 -08:00
Norman Maurer
e3cb9935c0 Take memory overhead of ChannelOutboundBuffer / PendingWriteQueue into account
Motivation:

To guard against the case that a user will enqueue a lot of empty or small buffers and so raise an OOME we need to also take the overhead of the ChannelOutboundBuffer / PendingWriteQueue into account when detect if a Channel is writable or not. This is related to #5856.

Modifications:

When calculate the memory for an message that is enqueued also add some extra bytes depending on the implementation.

Result:

Better guard against OOME.
2016-11-03 15:54:00 +01:00
Roger Kapsi
b2379e62f4 Allow customization of LocalChannel instances that are being created by LocalServerChannel.
Motivation

It's possible to extend LocalChannel as well as LocalServerChannel but the LocalServerChannel's serve(peer) method is hardcoded to create only instances of LocalChannel.

Modifications

Add a protected factory method that returns by default new LocalChannel(...) but users may override it to customize it.

Result

It's possible to customize the LocalChannel instance on either end of the virtual connection.
2016-10-30 08:32:49 +01:00
Norman Maurer
5f533b7358 [maven-release-plugin] prepare for next development iteration 2016-10-14 13:20:41 +02:00
Norman Maurer
35fb0babe2 [maven-release-plugin] prepare release netty-4.1.6.Final 2016-10-14 12:47:19 +02:00
Scott Mitchell
eb1dfb8f5e SingleThreadEventLoopTest failures
Motivation:
Some unit tests in SingleThreadEventLoopTest rely upon Thread.sleep for sequencing events between threads. This can be unreliable and result in spurious test failures if thread scheduling does not occur in a fair predictable manner.

Modifications:
- Reduce the reliance on Thread.sleep in SingleThreadEventLoopTest

Result:
Fixes https://github.com/netty/netty/issues/5851
2016-10-11 09:09:53 +02:00
Norman Maurer
6c8cd023bf [#5770] Use heapbuffers by default when using LocalChannel and LocalServerChannel.
Motivation:

The local transport is used to communicate in the same JVM so we should use heap buffers.

Modifications:

Use heapbuffers by default if not requested otherwise.

Result:

No allocating of direct buffers by default when using local transport
2016-10-10 11:16:17 +02:00
Norman Maurer
e102a008b6 [#5893] Ensure we not close NioDatagramChannel when SocketException is received.
Motivation:

When using java.nio.DatagramChannel we should not close the channel when a SocketException was thrown as we can still use the channel.

Modifications:

Not close the Channel when SocketException is thrown

Result:

More robust and correct handling of exceptions when using NioDatagramChannel.
2016-10-10 10:24:28 +02:00
Norman Maurer
a09e56850e [#5882] Ensure we even process tasks if processing of ready channels throws an Exception in event loop.
Motivation:

If an exception is thrown while processing the ready channels in the EventLoop we should still run all tasks as this may allow to recover. For example a OutOfMemoryError may be thrown and runAllTasks() will free up memory again. Beside this we should also ensure we always allow to shutdown even if an exception was thrown.

Modifications:

- Call runAllTasks() in a finally block
- Ensure shutdown is always handles.

Result:

More robust EventLoop implementations for NIO and Epoll.
2016-10-10 07:49:57 +02:00
Norman Maurer
3cf7ccbd3c Process OP_WRITE before OP_READ to free memory faster
Motivation:

We should better first process OP_WRITE before OP_READ as this may allow us to free memory in a faster fashion for previous queued writes.

Modifications:

Process OP_WRITE before OP_READ

Result:

Free memory faster for queued writes.
2016-10-10 07:42:39 +02:00
radai-rosenblatt
15ac6c4a1f Clean-up unused imports
Motivation:

the build doesnt seem to enforce this, so they piled up

Modifications:

removed unused import lines

Result:

less unused imports

Signed-off-by: radai-rosenblatt <radai.rosenblatt@gmail.com>
2016-09-30 09:08:50 +02:00
Roger Kapsi
149916d052 Adding ability omit the implicit #flush() call in EmbeddedChannel#writeOutbound() and
the implicit #fireChannelReadComplete() in EmbeddedChannel#writeInbound().

Motivation

We use EmbeddedChannels to implement a ProxyChannel of some sorts that shovels
messages between a source and a destination Channel. The latter are real network
channels (such as Epoll) and they may or may not be managed in a ChannelPool. We
could fuse both ends directly together but the EmbeddedChannel provides a nice
disposable section of a ChannelPipeline that can be used to instrument the messages
that are passing through the proxy portion.

The ideal flow looks abount like this:

source#channelRead() -> proxy#writeOutbound() -> destination#write()
source#channelReadComplete() -> proxy#flushOutbound() -> destination#flush()

destination#channelRead() -> proxy#writeInbound() -> source#write()
destination#channelReadComplete() -> proxy#flushInbound() -> source#flush()

The problem is that #writeOutbound() and #writeInbound() emit surplus #flush()
and #fireChannelReadComplete() events which in turn yield to surplus #flush()
calls on both ends of the pipeline.

Modifications

Introduce a new set of write methods that reain the same sematics as the #write()
method and #flushOutbound() and #flushInbound().

Result

It's possible to implement the above ideal flow.

Fix for EmbeddedChannel#ensureOpen() and Unit Tests for it

Some PR stuff.
2016-09-24 12:33:04 -07:00
ChuntaoLu
9008e72c2b Fix javadoc
Removes unmatched brace
2016-09-12 06:43:10 -07:00
Norman Maurer
6bbf32134a Log more details if notification of promise fails in PromiseNotifier and AbstractChannelHandlerContext
Motivation:

To make it easier to debug why notification of a promise failed we should log extra info and make it consistent.

Modifications:

- Create a new PromiseNotificationUtil that has static methods that can be used to try notify a promise and log.
- Reuse this in AbstractChannelHandlerContext, ChannelOutboundBuffer and PromiseNotifier

Result:

Easier to debug why a promise could not be notified.
2016-09-07 06:55:38 +02:00
Norman Maurer
dfa3bbbf00 Add support for Client Subnet in DNS Queries (RFC7871)
Motivation:

RFC7871 defines an extension which allows to request responses for a given subset.

Modifications:

- Add DnsOptPseudoRrRecord which can act as base class for extensions based on EDNS(0) as defined in RFC6891
- Add DnsOptEcsRecord to support the Client Subnet in DNS Queries extension
- Add tests

Result:

Client Subnet in DNS Queries extension is now supported.
2016-09-06 07:16:57 +02:00
buchgr
4af6855695 Remove @Deprecated for primitive WriteWaterMark getters and setters
Motivation:

For use cases that demand frequent updates of the write watermarks, an
API that requires immutable WriteWaterMark objects is not ideal, as it
implies a lot of object allocation.

For example, the HTTP/2 child channel API uses write watermarks for outbound
flow control and updates the write watermarks on every DATA frame write.

Modifications:

Remote @Deprecated tag from primitive getters and setters, however the corresponding
channel options remain deprecated.

Result:

Primitive getters and setters for write watermarks are no longer marked @Deprecated.
2016-09-05 10:26:05 +02:00
Norman Maurer
30fe2e868f Call finishConnect() before try to call read(...) / write(...) when using NIO
Motivation:

The JDK implementation of SocketChannel has an internal state that is tracked for its operations. Because of this we need to ensure we call finishConnect() before try to call read(...) / write(...) as otherwise it may produce a NotYetConnectedException.

Modifications:

First process OP_CONNECT flag.

Result:

No more possibility of NotYetConnectedException because OP_CONNECT is handled not early enough when processing interestedOps for a Channel.
2016-09-01 08:55:02 +02:00
Norman Maurer
54b1a100f4 [maven-release-plugin] prepare for next development iteration 2016-08-26 10:06:32 +02:00
Norman Maurer
6fd8bb8c63 [#5763] DefaultEventLoopGroup doesn't expose ctor variant that accepts custom Executor
Motivation:

The DefaultEventLoopGroup class extends MultithreadEventExecutorGroup but doesn't expose the ctor variants that accept a custom Executor like NioEventLoopGroup and EpollEventLoopGroup do.

Modifications:

Add missing constructor.

Result:

Be able to use custom Executor with DefaultEventLoopGroup.
2016-09-01 08:20:05 +02:00
Jason Tedor
00c0664ef8 Avoid inaccessible object exception replacing set
Motivation:

When attempting to set the selectedKeys fields on the selector
implementation, JDK 9 can throw an inaccessible object exception.

Modications:

Catch and log this exception as an possible course of action if the
sun.nio.ch package is not exported from java.base.

Result:

The selector replacement will fail gracefully as an expected course of
action if the sun.nio.ch package is not exported from java.base.
2016-08-31 13:59:48 +02:00
Norman Maurer
1208b90f57 [maven-release-plugin] prepare release netty-4.1.5.Final 2016-08-26 04:59:35 +02:00
Norman Maurer
d3cb95ef00 Make NIO and EPOLL transport connect errors more consistent with the JDK
Motivation:

The NIO transport used an IllegalStateException if a user tried to issue another connect(...) while the connect was still in process. For this case the JDK specified a ConnectPendingException which we should use. The same issues exists in the EPOLL transport. Beside this the EPOLL transport also does not throw the right exceptions for ENETUNREACH and EISCONN errno codes.

Modifications:

- Replace IllegalStateException with ConnectPendingException in NIO and EPOLL transport
- throw correct exceptions for ENETUNREACH and EISCONN in EPOLL transport
- Add test case

Result:

More correct error handling for connect attempts when using NIO and EPOLL transport
2016-08-27 20:57:36 +02:00
buchgr
4accd300e5 Fix write watermarks comparison to use less than and greater than.
Motivation:

The API documentation in ChannelConfig states that a a channel is writable,
if the number of pending bytes is below the low watermark and a
channel is not writable, if the number of pending bytes exceeds the high
watermark.

Therefore, we should use < operators instead of <= as well as > instead of >=.

Using <= and >= is also problematic, if the low watermark is equal to the high watermark,
as then a channel could be both writable and unwritable with the same number of pending
bytes (depending on whether remove() or addMessage() is called first).

The use of <= and >= was introduced in PR https://github.com/netty/netty/pull/3036, but
I don't understand why, as there doesn't seem to have been any discussion around that.

Modifications:

Use < and > operators instead of <= and >=.

Result:

High and low watermarks are treated as stated in the API docs.
2016-08-24 15:58:02 +02:00
Norman Maurer
5e148d5670 [#5639] Ensure fireChannelActive() is also called if Channel is closed in connect promise.
Motivation:

We need to ensure we also call fireChannelActive() if the Channel is directly closed in a ChannelFutureListener that is belongs to the promise for the connect. Otherwise we will see missing active events.

Modifications:

Ensure we always call fireChannelActive() if the Channel was active.

Result:

No missing events.
2016-08-24 08:47:49 +02:00
Norman Maurer
43626ac6ea Use NIO methods when using Java7+ in the NIO transport
Motivation:

We use often javachannel().socket().* in NIO as these methods exists in java6. The problem is that these will throw often very general Exceptions (Like SocketException) while it is more expected to throw the Exceptions listed in the nio interfaces. When possible we should use the new methods available in java7+ which throw the correct exceptions.

Modifications:

Check for java version and depending on it using the socket or the javachannel.

Result:

Throw expected Exceptions.
2016-08-24 07:36:14 +02:00
Norman Maurer
8ce7e73e78 Prevent extra peformance hit by fillInStackTrace() when create a new annotated connect exception.
Motivation:

To make it easier to debug connect exceptions we create new exceptions which also contain the remote address. For this we basically created a new instance and call setStackTrace(...). When doing this we pay an extra penality because it calls fillInStackTrace() when calling the super constructor.

Modifications:

Create special sub-classes of Exceptions that override the fillInStackTrace() method and so eliminate the overhead.

Result:

Less overhead when "annotate" connect exceptions.
2016-08-24 07:34:43 +02:00
buchgr
8d1e46ffd1 Remove reference to 5.0 release.
Motivation:

Comments stating that AUTO_CLOSE will be removed in Netty 5.0 are wrong,
as there is no Netty 5.0.

Modifications:

Removed comment.

Result:

No more references to Netty 5.0
2016-08-23 09:46:57 +02:00
Norman Maurer
6d70f4b38a Guard against re-entrance in PendingWriteQueue.removeAndWriteAll()
Motivation:

PendingWriteQueue should guard against re-entrant writes once removeAndWriteAll() is run.

Modifications:

Continue writing until queue is empty.

Result:

Correctly guard against re-entrance.
2016-08-18 07:13:19 +02:00
tbcs
95ee705e17 fix typo in javadoc 2016-08-12 20:42:37 +02:00
Jason Tedor
a6dfd08812 Mark initialization of selector as privileged
Motivation:

Instrumenting the NIO selector implementation requires special
permissions. Yet, the code for performing this instrumentation is
executed in a manner that would require all code leading up to the
initialization to have the requisite permissions. In a restrictive
environment (e.g., under a security policy that only grants the
requisite permissions the Netty transport jar but not to application
code triggering the Netty initialization), then instrumeting the
selector will not succeed even if the security policy would otherwise
permit it.

Modifications:

This commit marks the necessary blocks as privileged. This enables
access to the necessary resources for instrumenting the selector. The
idea is that we are saying the Netty code is trusted, and as long as the
Netty code has been granted the necessary permissions, then we will
allow the caller access to these resources even though the caller itself
might not have the requisite permissions.

Result:

The selector can be instrumented in a restrictive security environment.
2016-08-05 19:01:57 +02:00
Jason Tedor
32629078a2 Mark setting of sun.nio.ch.bugLevel as privileged
Motivation:

Writing to a system property requires permissions. Yet the code for
setting sun.nio.ch.bugLevel is not marked as privileged. In a
restrictive environment (e.g., under a security policy that only grants
the requisite permissions the Netty transport jar but not to application
code triggering the Netty initialization), writing to this system
property will not succeed even if the security policy would otherwise
permit it.

Modifications:

This commt marks the necessary code block as privileged. This enables
writing to this system property. The idea is that we are saying the
Netty code is trusted, and as long as the Netty code has been granted
the necessary permissions, then we will allow the caller access to these
resources even though the caller itself might not have the requisite
permissions.

Result:

The system property sun.nio.ch.bugLevel can be written to in a
restrictive security environment.
2016-08-05 18:58:34 +02:00
Norman Maurer
aa6e6ae307 [#4241] Ensure NioEventLoopGroup.shutdownGracefully(...) with no quiet period shutdown as fast as expected.
Motivation:

If the user uses 0 as quiet period we should shutdown without any delay if possible.

Modifications:

Ensure we not introduce extra delay when a shutdown quit period of 0 is used.

Result:

EventLoop shutdown as fast as expected.
2016-08-05 07:21:17 +02:00
Norman Maurer
26aa34853a Ensure correct ordering if a ChannelInitializer adds another ChannelInitializer
Motivation:

At the moment we call initChannel(...) in the channelRegistered(...) method which has the effect that if another ChannelInitializer is added within the initChannel(...) method the ordering of the added handlers is not correct and surprising. This is as the whole initChannel(...) method block is executed before the initChannel(...) block of the added ChannelInitializer is handled.

Modifications:

Call initChannel(...) from within handlerAdded(...) if the Channel is registered already. This is true in all cases for our DefaultChannelPipeline implementation. This way the ordering is always as expected. We still keep the old behaviour as well to not break code for other ChannelPipeline implementations (if someone ever wrote one).

Result:

Correct and expected ordering of ChannelHandlers.
2016-08-03 07:50:46 +02:00
Norman Maurer
f585806a74 [#5598] Ensure SslHandler not log false-positives when try to close the channel due timeout.
Motivation:

When we try to close the Channel due a timeout we need to ensure we not log if the notification of the promise fails as it may be completed in the meantime.

Modifications:

Add another constructor to ChannelPromiseNotifier and PromiseNotifier which allows to log on notification failure.

Result:

No more miss-leading logs.
2016-07-30 21:15:09 +02:00
Norman Maurer
cb7cf4491c [maven-release-plugin] prepare for next development iteration 2016-07-27 13:29:56 +02:00
Norman Maurer
9466b32d05 [maven-release-plugin] prepare release netty-4.1.4.Final 2016-07-27 13:16:59 +02:00
Norman Maurer
b97a36a10f Add test to verify that its possible to add another ChannelInitializer in the initChannel(...) method.
Motivation:

I received a report the its not possible to add another ChannelInitialiter in the initChannel(...) method, so we should add a test case for it.

Modifications:

Added testcase.

Result:

Validate that all works as expected.
2016-07-27 09:25:13 +02:00
Norman Maurer
4638df2062 [#5566] Ensure using a ChannelInitializer via ServerBootstrap.handler(...) produce correct ordering.
Motivation:

When a ChannelInitializer is used via ServerBootstrap.handler(...) the users handlers may be added after the internal ServerBootstrapAcceptor. This should not happen.

Modifications:

Delay the adding of the ServerBootstrapAcceptor until the initChannel(....) method returns.

Result:

Correct order of handlers in the ServerChannels ChannelPipeline.
2016-07-27 08:34:31 +02:00
Norman Maurer
dc6c6d956a [#5541] Ensure failing a Promise in SimpleChannelPool will not result in stack overflow.
Motivation:

We used Promise.setFailure(...) when fail a Promise in SimpleChannelPool. As this happens in multiple levels this can result in stackoverflow as setFailure(...) may throw an IllegalStateException which then again is propergated.

Modifications:

Use tryFailure(...)

Result:

No more possibility to cause a stack overflow when failing the promise.
2016-07-27 07:57:58 +02:00
Norman Maurer
b8400f9628 [#5553] SimpleChannelPool#notifyConnect() may leak Channels
Motivation:

The SimpleChannelPool#notifyConnect() method will leak Channels if the user cancelled the Promise in between.

Modifications:

Release the channel if the Promise was complete before.

Result:

No more channel leaks.
2016-07-20 20:17:19 +02:00
Jason Tedor
d262f7c189 Reduce permissions needed for process ID
Motiviation:

DefaultChannelId attempts to acquire a default process ID by determining
the process PID. However, to do this it attempts to punch through to the
system classloader, a permission that in the face of a restrictive
security manager is unlikely to be granted. Looking past this, it then
attempts to load a declared method off a reflectively loaded class,
another permission that is not likely to be granted in the face of a
restrictive security manager. However, neither of these permissions are
necessary as the punching through to the system security manager is
completely unneeded, and there is no need to load a public method as a
declared method.

Modifications:

Instead of punching through to the system classloader requiring
restricted permissions, we can just use current classloader. To address
the access declared method permission, we instead just reflectively
obtain the desired public method via Class#getMethod.

Result:

Acquiring the default process ID from the PID will succeed without
requiring the runtime permissions "getClassLoader" and
"accessDeclaredMembers".
2016-07-20 19:47:56 +02:00
Scott Mitchell
3d7ae97359 Make Epoll ChannelMetadata more consistent with NIO
Motivation:
In 4.0 AbstractNioByteChannel has a default of 16 max messages per read. However in 4.1 that constraint was applied at the NioSocketChannel which is not equivalent. In 4.1 AbstractEpollStreamChannel also did not have the default of 16 max messages per read applied.

Modifications:
- Make Nio consistent with 4.0
- Make Epoll consistent with Nio

Result:
Nio and Epoll both have consistent ChannelMetadata and are consistent with 4.0.
2016-07-18 13:26:05 +02:00
Norman Maurer
047f6aed28 [maven-release-plugin] prepare for next development iteration 2016-07-15 09:09:13 +02:00
Norman Maurer
b2adea87a0 [maven-release-plugin] prepare release netty-4.1.3.Final 2016-07-15 09:08:53 +02:00
Nitesh Kant
77770374fb Ability to run a task at the end of an eventloop iteration.
Motivation:

This change is part of the change done in PR #5395 to provide an `AUTO_FLUSH` capability.
Splitting this change will enable to try other ways of implementing `AUTO_FLUSH`.

Modifications:



Two methods:

```java
void executeAfterEventLoopIteration(Runnable task);


boolean removeAfterEventLoopIterationTask(Runnable task);
```
are added to `SingleThreadEventLoop` class for adding/removing a task to be executed at the end of current/next iteration of this `eventloop`.

In order to support the above, a few methods are added to `SingleThreadEventExecutor`

```java
protected void afterRunningAllTasks() { }
```

This is invoked after all tasks are run for this executor OR if the passed timeout value for `runAllTasks(long timeoutNanos)` is expired.

Added a queue of `tailTasks` to `SingleThreadEventLoop` to hold all tasks to be executed at the end of every iteration.


Result:



`SingleThreadEventLoop` now has the ability to execute tasks at the end of an eventloop iteration.
2016-07-12 10:22:15 +02:00
Norman Maurer
b37a41a535 Allow to get the number of bytes queued in PendingWriteQueue
Motivation:

For some use-cases it would be useful to know the number of bytes queued in the PendingWriteQueue without the need to dequeue them.

Modifications:

Add PendingWriteQueue.bytes().

Result:

Be able to get the number of bytes queued.
2016-07-11 09:05:20 +02:00
Norman Maurer
50a74e95f2 Ensure ChannelHandler.handlerAdded(...) callback is executed directly when added from ChannelFutureListener added to the registration future.
Motivation:

Commit 4c048d069d moved the logic of calling handlerAdded(...) to the channelRegistered(...) callback of the head of the DefaultChannelPipeline. Unfortunatlly this may execute the callbacks to late as a user may add handlers to the pipeline in the ChannelFutureListener attached to the registration future. This can lead to incorrect ordering.

Modifications:

Ensure we always invoke ChannelHandler.handlerAdded(...) for all handlers before the registration promise is notified.

Result:

Not possible of incorrect ordering or missed events.
2016-07-09 08:04:15 +02:00
Norman Maurer
061899f2a7 Allow to remove pinning of EventExecutor for EventExecutorGroup
Motivation:

We pinned the EventExecutor for a Channel in DefaultChannelPipeline. Which means if the user added multiple handlers with the same EventExecutorGroup to the ChannelPipeline it will use the same EventExecutor for all of these handlers. This may be unexpected and even not what the user wants. If the user want to use the same one for all of them it can be done by obtain an EventExecutor and pass the same instance to the add methods. Because of this we should allow to not pin.

Modifications:

Allow to disable pinning of EventExecutor for Channel based on EventExecutorGroup via ChannelOption.

Result:

Less confusing and more flexible usage of EventExecutorGroup when adding ChannelHandlers to the ChannelPipeline.
2016-07-08 20:09:16 +02:00
Norman Maurer
64bf167423 Fix flacky test introducd by 29fdb160f3 2016-07-07 22:53:08 +02:00
Roger Kapsi
02850da480 Rename ChannelHandlerContext#fireUserEventTriggered() argument from event to evt so it matches the ChannelInboundHandler#userEventTriggered() argument's name.
Motivation

When I override ChannelHandler methods I usually (always) refire events myself via
ChannelHandlerContext instead of relieing on calling the super method (say
`super.write(ctx, ...)`). This works great and the IDE actually auto completes/generates
the right code for it except `#fireUserEventTriggered()` and `#userEventTriggered()`
which have a mismatching argument names and I have to manually "intervene".

Modification

Rename `ChannelHandlerContext#fireUserEventTriggered()` argument from `event` to `evt`
to match its handler counterpart.

Result

The IDE's auto generated code will reference the correct variable.
2016-07-07 17:01:19 +02:00
Norman Maurer
29fdb160f3 [#5486] Not operate on serial execution assumption when using EventExecutor in the DefaultChannelPipeline.
Motivation:

In commit f984870ccc I made a change which operated under invalide assumption that tasks executed by an EventExecutor will always be processed in a serial fashion. This is true for SingleThreadEventExecutor sub-classes but not part of the EventExecutor interface contract.

Because of this change implementations of EventExecutor which not strictly execute tasks in a serial fashion may miss events before handlerAdded(...) is called. This is strictly speaking not correct as there is not guarantee in this case that handlerAdded(...) will be called as first task (as there is no ordering guarentee).

Cassandra itself ships such an EventExecutor implementation which has no strict ordering to spread load across multiple threads.

Modifications:

- Add new OrderedEventExecutor interface and let SingleThreadEventExecutor / EventLoop implement / extend it.
- Only expose "restriction" of skipping events until handlerAdded(...) is called for OrderedEventExecutor implementations
- Add ThreadPoolEventExecutor implementation which executes tasks in an unordered fashion. This is used in added unit test but can also be used for protocols which not expose an strict ordering.
- Add unit test.

Result:

Resurrect the possibility to implement an EventExecutor which does not enforce serial execution of events and be able to use it with the DefaultChannelPipeline.
2016-07-07 15:01:56 +02:00
Norman Maurer
c393374cf5 [#5455] Clarify ChannelPool javadocs
Motivation:

We should make it clear that each acquired Channel needs to be released in all cases.

Modifications:

More clear javadocs.

Result:

Harder for users to leak Channel.
2016-07-07 06:46:28 +02:00
Norman Maurer
195d7476d5 Deprecate methods in AbstractChannel that have no real usage.
Motivation:

We should deprecate methods that are not used.

Modifications:

Add @Deprecated to methods

Result:

Be able to cleanup stuff sooner.
2016-07-07 06:41:43 +02:00
buchgr
0fbb791ad6 Make AbstractChannel.outboundBuffer volatile.
Motivation:

The field can be read from arbitrary threads via Channel.(isWritable()|bytesBeforeWritable()|bytesBeforeUnwritable()), WriteAndFlushTask.newInstance(), PendingWriteQueue, etc.

Modifications:

Make AbstractChannel.outboundBuffer volatile.

Result:

More correct in a concurrent use case.
2016-07-06 21:25:33 +02:00
Norman Maurer
44b942e507 Rename future to promise in the ChannelDuplexHandler method arguments.
Motivation:

We used future in many method of ChannelDuplexHandler as argument name of ChannelPromise. We should make it more consistent and correct.

Modifications:

Replace future with promise.

Result:

More correct and consistent naming.
2016-07-06 08:55:29 +02:00
Norman Maurer
4676a2271c [maven-release-plugin] prepare for next development iteration 2016-07-01 10:33:32 +02:00
Norman Maurer
ad270c02b9 [maven-release-plugin] prepare release netty-4.1.2.Final 2016-07-01 09:07:40 +02:00
Norman Maurer
731f52fdf7 Allow to inject RejectedExecutionHandler for different EventLoops and EventExecutors
Motiviation:

Sometimes it is useful to allow to specify a custom strategy to handle rejected tasks. For example if someone tries to add tasks from outside the eventloop it may make sense to try to backoff and retries and so give the executor time to recover.

Modification:

Add RejectedEventExecutor interface and implementations and allow to inject it.

Result:

More flexible handling of executor overload.
2016-06-24 17:08:30 +02:00
Norman Maurer
a0d4fb16fc Allow to set max capacity for task queue for EventExecutors and EventLoops
Motivation:

To restrict the memory usage of a system it is sometimes needed to adjust the number of max pending tasks in the tasks queue.

Modifications:

- Add new constructors to modify the number of allowed pending tasks.
- Add system properties to configure the default values.

Result:

More flexible configuration.
2016-06-24 14:00:50 +02:00
Norman Maurer
278c36af0c Merge ThrowableUtils into ThrowableUtil.
Motivation:

We should merge ThrowableUtils into ThrowableUtil as this name is more consistent with the naming of utility classes in netty.

Modifications:

Merge classes.

Result:

More consistent naming
2016-06-23 10:05:41 +02:00
Tim Brooks
d964bf6f18 Remove usages of deprecated methods group() and childGroup().
Motivation:

These methods were recently deprecated. However, they remained in use in several locations in Netty's codebase.

Modifications:

Netty's code will now access the bootstrap config to get the group or child group.

Result:

No impact on functionality.
2016-06-21 14:06:57 +02:00
Norman Maurer
31e0ac1d22 Expose the SelectorProvider used by the NioEventLoop
Motivation:

If a user writes an own nio based transport which uses a special SelectorProvider it is useful to be able to get the SelectorProvider that is used by a NioEventLoop. This way this can be used when implement AbstractChannel.isCompatible(...) and check that the SelectorProvider is the correct one.

Modifications:

Expose the SelectorProvider.

Result:

Be able to get the SelectorProvider used by a NioEventLoop.
2016-06-21 14:05:44 +02:00
Norman Maurer
e845670043 Set some StackTraceElement on pre-instantiated static exceptions
Motivation:

We use pre-instantiated exceptions in various places for performance reasons. These exceptions don't include a stacktrace which makes it hard to know where the exception was thrown. This is especially true as we use the same exception type (for example ChannelClosedException) in different places. Setting some StackTraceElements will provide more context as to where these exceptions original and make debugging easier.

Modifications:

Set a generated StackTraceElement on these pre-instantiated exceptions which at least contains the origin class and method name. The filename and linenumber are specified as unkown (as stated in the javadocs of StackTraceElement).

Result:

Easier to find the origin of a pre-instantiated exception.
2016-06-20 11:33:05 +02:00
Norman Maurer
8f3a5e5b18 Fix typo in log message
Motivation:

We had a typo in the log message.

Modifications:

Remove extra "s" in log message.

Result:

Correct spelling in log message.
2016-06-17 06:23:00 +02:00
Norman Maurer
418550914a Log Selector instance when Selector needs to be rebuild
Motivation:

To better debug why a Selector need to be rebuild it is useful to also log the instance of the Selector.

Modifications:

Add logger instance to the log message.

Result:

More useful log message.
2016-06-17 06:19:23 +02:00
Stephane Landelle
9bfeab2c8a AbstractBootstrap can crash instead of failing promise, close #5387
Motivation:
When `ChannelFactory#newChannel` crashed, `AbstractBootstrap#initAndRegister` propagates the exception to the caller instead of failing the promise.

Modifications:
- Catch exceptions from `ChannelFactory#newChannel`.
- Notify promise of such failure.

Result:
`AbstractBootstrap` gracefully handles connect failures.
2016-06-13 18:59:09 +02:00
Dmitry Spikhalskiy
428c61673b Logs in invokeExceptionCaught have been made consistent and full
Motivation:

In case of exception in invokeExceptionCaught() only original exception passed to invokeExceptionCaught() will be logged on any log level.
+ AbstractChannelHandlerContext and CombinedChannelDuplexHandler log different exceptions.

Modifications:

Fix inconsistent logging code and add ability to see both stacktraces on DEBUG level.

Result:

Both handlers log now both original exception and thrown from invokeExceptionCaught. To see full stacktrace of exception thrown from invokeExceptionCaught DEBUG log level must be enabled.
2016-06-11 20:11:11 +02:00
Guido Medina
c3abb9146e Use shaded dependency on JCTools instead of copy and paste
Motivation:
JCTools supports both non-unsafe, unsafe versions of queues and JDK6 which allows us to shade the library in netty-common allowing it to stay "zero dependency".

Modifications:
- Remove copy paste JCTools code and shade the library (dependencies that are shaded should be removed from the <dependencies> section of the generated POM).
- Remove usage of OneTimeTask and remove it all together.

Result:
Less code to maintain and easier to update JCTools and less GC pressure as the queue implementation nt creates so much garbage
2016-06-10 13:19:45 +02:00
Jon Chambers
829be86223 Use a default resolver with bootstrap.resolver(null).
Motivation:

`Bootstrap` has a notion of a default resolver group, but it's hidden from the public. To allow callers to reset a `Bootstrap` instance's resolver group, we could either make `DEFAULT_RESOLVER` public, or we could allow callers to pass `null` as an argument to `Bootstrap#resolver(AddressResolverGroup<?>)`. This pull request does the latter.

Modifications:

- Allow `Bootstrap#resolver(AddressResolverGroup<?>)` to accept `null` as an argument

Result:

Callers may pass `null` to `Bootstrap#resolver(AddressResolverGroup<?>)` to cause the `Bootstrap` instance to use its default resolver group.
2016-06-08 06:57:55 +02:00
Norman Maurer
4dec7f11b7 [maven-release-plugin] prepare for next development iteration 2016-06-07 18:52:34 +02:00
Norman Maurer
cf670fab75 [maven-release-plugin] prepare release netty-4.1.1.Final 2016-06-07 18:52:22 +02:00
Yuri Schimke
a14eda7db0 typo: Skelton 2016-06-06 16:28:21 -07:00
Norman Maurer
b461c9d54c Allow to specify a custom EventExecutorChooserFactory. Related to [#1230]
Motivation:

Sometimes it may be benefitially for an user to specify a custom algorithm when choose the next EventExecutor/EventLoop.

Modifications:

Allow to specify a custom EventExecutorChooseFactory that allows to customize algorithm.

Result:

More flexible api.
2016-06-06 11:04:56 +02:00
Norman Maurer
e847ac0443 Fix possible deadlock in DefaultChannelPipeline.destroyDown(...)
Motivation:

We need to ensure we not hold a lock while executor callHandlerRemoved(...) as this may lead to a deadlock if handlerRemoved(...) will call another method in DEfaultChannelPipeline from another thread that will need to obtain the lock as well and wait for the result.

Modifications:

Release the lock before call handlerRemoved0(...).

Result:

No more deadlock possible
2016-06-04 09:13:54 +02:00
Norman Maurer
f8b306f61c [#5313] Correctly catch errors during bootstrap.
Motivation:

We not correctly catched errors during resolving in bootstrap and so may not have notified the future correctly.

Modifications:

Move code into try / catch block and try to fail the promise.

Result:

Promise is always notified
2016-06-04 09:12:54 +02:00
Norman Maurer
c43b424f56 Add missing null check that was missed in 844976a0a2 2016-05-31 14:05:03 +02:00
Norman Maurer
844976a0a2 Ensure the same ByteBufAllocator is used in the EmbeddedChannel when compress / decompress. Related to [#5294]
Motivation:

The user may specify to use a different allocator then the default. In this case we need to ensure it is shared when creating the EmbeddedChannel inside of a ChannelHandler

Modifications:

Use the config of the "original" Channel in the EmbeddedChannel and so share the same allocator etc.

Result:

Same type of buffers are used.
2016-05-31 09:08:33 +02:00
Norman Maurer
339b512e70 Fix small race in DefaultChannelPipeline introduced by a729e0fcd9
Motivation:

There is a small race while adding handlers to the pipeline because callHandlerAddedForAllHandlers() may not be run when the user calls add* but the Channel is already registered.

Modifications:

Ensure we always delay handlerAdded(..) / handlerRemoved(...) until callHandlerAddedForAllHandlers() was called.

Result:

No more race on pipeline modifications possible.
2016-05-30 15:05:48 +02:00
Norman Maurer
dcd93e3be0 Remove volatile where not needed.
Motivation:

We can remove the volatile keyword from the cached Runnables as at worse these will just be re-created.

Modifications:

Remove volatile.

Result:

Less overhead.
2016-05-30 07:32:45 +02:00
Norman Maurer
86f53083e7 [#5297] Ensure calling NioEventLoop.pendingTasks() and EpollEventLoop.pendingTasks() will not produce livelock
Motivation:

SingleThreadEventExecutor.pendingTasks() will call taskQueue.size() to get the number of pending tasks in the queue. This is not safe when using MpscLinkedQueue as size() is only allowed to be called by a single consumer.

Modifications:

Ensure size() is only called from the EventLoop.

Result:

No more livelock possible when call pendingTasks, no matter from which thread it is done.
2016-05-28 20:04:07 +02:00
Norman Maurer
6ca49d1336 [maven-release-plugin] prepare for next development iteration 2016-05-25 19:16:44 +02:00
Norman Maurer
446b38db52 [maven-release-plugin] prepare release netty-4.1.0.Final 2016-05-25 19:14:15 +02:00
Norman Maurer
2618c2a649 [#5239] Allow adding handlers to pipeline with null name.
Motivation:

While doing 8fe3c83e4c I made a change which disallowed using null as name for handlers in the pipeline (this generated a new name before).

Modifications:

Revert to old behaviour and adding test case.

Result:

Allow null name again
2016-05-24 06:22:44 +02:00
Norman Maurer
26a175cd94 Fix flacky test which was missed when commit 8fe3c83e4c 2016-05-22 19:33:48 +02:00
Norman Maurer
7b25402e80 Add CompositeByteBuf.addComponent(boolean ...) method to simplify usage
Motivation:

At the moment the user is responsible to increase the writer index of the composite buffer when a new component is added. We should add some methods that handle this for the user as this is the most popular usage of the composite buffer.

Modifications:

Add new methods that autoamtically increase the writerIndex when buffers are added.

Result:

Easier usage of CompositeByteBuf.
2016-05-21 19:52:16 +02:00
Xiaoyan Lin
a5006c1969 Add a EventLoopGroup.register(ChannelPromise)
Motivation:

EventLoopGroup.register doesn't need the Channel paramter when ChannelPromise is provided as we can get the Channel from ChannelPromise. Resolves #2422.

Modifications:

- Add EventLoopGroup.register(ChannelPromise)
- Deprecate EventLoopGroup.register(Channel, ChannelPromise)

Result:

EventLoopGroup.register is more convenient as people only need to set one parameter.
2016-05-21 18:40:17 +02:00
Norman Maurer
a729e0fcd9 Decouple DefaultChannelPipeline from AbstractChannel
Motivation:

DefaultChannelPipeline was tightly coupled to AbstractChannel which is not really needed.

Modifications:

Move logic of calling handlerAdded(...) for handlers that were added before the Channel was registered to DefaultChannelPipeline by making it part of the head context.

Result:

Less coupling and so be able to use DefaultChannelPipeline also with other Channel implementations that not extend AbstractChannel
2016-05-21 17:14:00 +02:00
Norman Maurer
0838f223e1 Decouple AbstractChannel and AbstractChannelHandlerContext
Motivation:

We do a "blind" cast to AbstractChannel in AbstractChannelHandlerContext which we should better no do. It would be better to decouble AbstractChannelHandlerContext from AbstractChannel.

Modifications:

Decouble AbstractChannelHandlerContext from AbstractChannel by move logic to DefaultChannelPipeline

Result:

Less coubling and less casting.
2016-05-21 10:46:21 +02:00
Norman Maurer
7547a448e0 [#4906] Ensure addLast(...) works as expected in EmbeddedChannel
Motivation:

If the user will use addLast(...) on the ChannelPipeline of EmbeddedChannel after its constructor was run it will break the EmbeddedChannel as it will not be able to collect inbound messages and exceptions.

Modifications:

Ensure addLast(...) work as expected by move the logic of handling messages and exceptions ti protected methods of DefaultChannelPipeline and use a custom implementation for EmbeddedChannel

Result:

addLast(...) works as expected when using EmbeddedChannel.
2016-05-21 10:33:25 +02:00
Norman Maurer
e10dca7601 Mark Recycler.recycle(...) deprecated and update usage.
Motivation:

Recycler.recycle(...) should not be used anymore and be replaced by Handle.recycle().

Modifications:

Mark it as deprecated and update usage.

Result:

Correctly document deprecated api.
2016-05-20 22:11:31 +02:00
Norman Maurer
2e352b75ac Remove unused class
Motivation:

We should remove unused classes that are package-private

Modifications:

Remove unused class

Result:

Less cruft in code-base.
2016-05-20 21:53:59 +02:00
Norman Maurer
a425a8551d [#5174] Expose Bootstrap getter methods and add some additional ones
Motivation:

The Bootstrap class (applies also to AbstractBootstrap and ServerBootstrap) has a few package private getter methods and some things such as #attr() and #options() aren't exposed at all.

Modifications:

Expose "getters" for configured things in a safe-manner via the config() method.

Result:

Easier for the user to check what is configured for a Bootstrap/ServerBootstrap.
2016-05-18 13:26:43 +02:00