Commit Graph

1634 Commits

Author SHA1 Message Date
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
Scott Mitchell
2b340df452 DuplexChannel to support shutdownInput
Motivation:
The DuplexChannel is currently incomplete and only supports shutting down the output side of a channel. This interface should also support shutting down the input side of the channel.

Modifications:
- Add shutdownInput and shutdown methods to the DuplexChannel interface
- Remove state in NIO and OIO for tracking input being shutdown independent of the underlying transport's socket type. Tracking the state independently may lead to inconsistent state.

Result:
DuplexChannel supports shutting down the input side of the channel
Fixes https://github.com/netty/netty/issues/5175
2016-05-18 09:11:49 +02:00
Norman Maurer
8fe3c83e4c [#5104] Fix possible deadlock in DefaultChannelPipeline
Motivation:

When a user has multiple EventLoops in an EventLoopGroup and calls pipeline.add* / remove* / replace from an EventLoop that belongs to another Channel it is possible to deadlock if the other EventLoop does the same.

Modification:

- Only ensure the actual modification takes place in a synchronized block and not wait until the handlerAdded(...) / handlerRemoved(...) method is called. This is ok as we submit the task to the executor while still holding the look and so ensure correct order of pipeline modifications.
- Ensure if an AbstractChannelHandlerContext is put in the linked-list structure but the handlerAdded(...) method was not called we skip it until handlerAdded(...) was called. This is needed to ensure handlerAdded(...) is always called first.

Result:

Its not possible to deadlock when modify the DefaultChannelPipeline.
2016-05-17 14:24:46 +02:00
Trustin Lee
3a9f472161 Make retained derived buffers recyclable
Related: #4333 #4421 #5128

Motivation:

slice(), duplicate() and readSlice() currently create a non-recyclable
derived buffer instance. Under heavy load, an application that creates a
lot of derived buffers can put the garbage collector under pressure.

Modifications:

- Add the following methods which creates a non-recyclable derived buffer
  - retainedSlice()
  - retainedDuplicate()
  - readRetainedSlice()
- Add the new recyclable derived buffer implementations, which has its
  own reference count value
- Add ByteBufHolder.retainedDuplicate()
- Add ByteBufHolder.replace(ByteBuf) so that..
  - a user can replace the content of the holder in a consistent way
  - copy/duplicate/retainedDuplicate() can delegate the holder
    construction to replace(ByteBuf)
- Use retainedDuplicate() and retainedSlice() wherever possible
- Miscellaneous:
  - Rename DuplicateByteBufTest to DuplicatedByteBufTest (missing 'D')
  - Make ReplayingDecoderByteBuf.reject() return an exception instead of
    throwing it so that its callers don't need to add dummy return
    statement

Result:

Derived buffers are now recycled when created via retainedSlice() and
retainedDuplicate() and derived from a pooled buffer
2016-05-17 11:16:13 +02:00
Norman Maurer
68cd670eb9 Remove ChannelHandlerInvoker
Motivation:

We tried to provide the ability for the user to change the semantics of the threading-model by delegate the invoking of the ChannelHandler to the ChannelHandlerInvoker. Unfortunually this not really worked out quite well and resulted in just more complexity and splitting of code that belongs together. We should remove the ChannelHandlerInvoker again and just do the same as in 4.0

Modifications:

Remove ChannelHandlerInvoker again and replace its usage in Http2MultiplexCodec

Result:

Easier code and less bad abstractions.
2016-05-17 11:14:00 +02:00
Hyangtack Lee
f44f3e7926 NioEventLoop ensures that all submitted tasks are executed immediately.
Motivation:
If a task was submitted when wakenUp value was true, the task didn't get a chance to call Selector#wakeup.
So we need to check task queue again before executing select operation. If we don't, the task might be pended until select operation was timed out.
It might be pended until idle timeout if IdleStateHandler existed in pipeline.

Modifications:
Execute Selector#select in a non-blocking manner if there's a task submitted when wakenUp value was true.

Result:
Every tasks in NioEventLoop will not be pended.
2016-05-17 07:43:46 +02:00
Norman Maurer
c249926784 [#3127] Allow to write with VoidPromise to Channels in ChannelGroup
Motivation:

Users sometimes want to use Channel.voidPromise() when write to a Channel to reduce GC-pressure. This should be also possible when write via a ChannelGroup.

Modifications:

Add new write(...) and writeAndFlush(...) overloads which allow to signale that a VoidPromise should be used to write to the Channel

Result:

Users can write with VoidPromise when using ChannelGroup
2016-05-14 20:53:25 +02:00
Norman Maurer
27a392b877 Add ChannelInboundInvoker and ChannelOutboundInvoker
Motivation:

ChannelHandlerContext, ChannelPipeline and Channel share various method signatures. We should provide an interface to share.

Modifications:

Add ChannelInboundInvoker and ChannelOutboundInvoker and extend these.

Result:

Better API abstraction.
2016-05-14 20:41:13 +02:00
Norman Maurer
01109dd635 Fix test-failures introduces by c1827114e9. 2016-05-14 20:38:31 +02:00
Norman Maurer
c1827114e9 Use ConnectException when connection failed for LocalChannel
Motivation:

To be more consistent we should use ConnectException when we fail the connect attempt because no LocalServerChannel exists with the given address.

Modifications:

Use correct exception.

Result:

More consistent handling of connection refused between different transports.
2016-05-14 07:23:55 +02:00
Norman Maurer
775dd139ea Ensure Bootstrap.connect(...) not throw IllegalStateException when registration is delayed.
Motivation:

Bootstrap.connect(...) tries to obtain the EventLoop of a Channel before it may be registered. This will cause an IllegalStateException. We need to ensure we handle the cause of late registration and not throw in this case.

Modifications:

Ensure we only try to access the EventLoop after the Channel is registered and handle the case of late registration.

Result:

Bootstrap.connect(...) not fails on late registration.
2016-05-14 07:22:38 +02:00
Scott Mitchell
1f8e192e21 Remove EventExecutor.children
Motivation:
EventExecutor.children uses generics in such a way that an entire colleciton must be cast to a specific type of object. This interface is not very flexible and is impossible to implement if the EventExecutor type must be wrapped. The current usage of this method also does not have any clear need within Netty. The Iterator interface allows for EventExecutor to be wrapped and forces the caller to make assumptions about types instead of building the assumptions into the interface.

Motivation:
- Remove EventExecutor.children and undeprecate the iterator() interface

Result:
EventExecutor interface has one less method and is easier to wrap.
2016-05-13 18:17:22 -07:00
Norman Maurer
690ab563e7 Ensure ChannelHandlerContext.attr(...) and ChannelHandlerContext.hasAttr(...) has no semantic change
Motivation:

The ChannelHandlerContext.attr(...) and ChannelHandlerContext.hasAttr(...) delegated to Channel for the attributes which is a semantic change compared to 4.0 releases. We should not change the semantic to not break users applications when upgrading to 4.1.0

Modifications:

- Revert semantic change
- Mark ChannelHandlerContext.attr(...) and hasAttr(...) as @deprecated so we can remove these later

Result:

Semantic of attribute operations on ChannelHandlerContext is the same in 4.1 as in 4.0 again.
2016-05-13 15:51:58 +02:00
Norman Maurer
5ffa3b1c46 Only try to bind if late registration not failed.
Motivation:

We should not try to call bind if registration failed.

Modifications:

Only call doBind0(...) when the registration not failed.

Result:

Not try to to bind if the registration failed.
2016-05-13 12:10:31 +02:00
Norman Maurer
ea61b5b5b3 [#5223] Remove indirection in DefaultChannelPipeline.executorSafe(...)
Motivation:

We use channel.unsafe().invoker().executor() in DefaultChannelPipeline.executorSafe(...) which is an unnecessary indirection to channel.eventLoop().

Modifications:

Remove indirection by using channel.eventLoop().

Result:

Cleaner code.
2016-05-13 08:42:36 +02:00
Scott Mitchell
f2ed3e6ce8 DefaultPromise LateListener Logic Issues
Motivation:
The LateListener logic is prone to infinite loops and relies on being processed in the EventExecutor's thread for synchronization, but this EventExecutor may not be constant. An infinite loop can occur if the EventExecutor's execute method does not introduce a context switch in LateListener.run. The EventExecutor can be changed by classes which inherit from DefaultPromise. For example the DefaultChannelPromise will return w/e EventLoop the channel is registered to, but this EventLoop can change (re-registration).

Modifications:
- Remove the LateListener concept and instead use a single Object to maintain the listeners while still preserving notification order
- Make the result member variable an atomic variable so it can be outside the synchronized(this) blocks
- Cleanup/simplify existing state management code

Result:
Fixes https://github.com/netty/netty/issues/5185
2016-05-09 10:33:40 -07:00
Norman Maurer
b2bb72b2de [#5171] Ensure NioDatagramChannelConfig can be instanced on android
Motivation:

NioDatagramChannelConfig currently uses NetworkChannel in its static { } block and so fails to init on android which not has this class.

Modifications:

Use reflection to load the NetworkChannel.class

Result:

Be able to use NIO Datagram on android as well.
2016-05-06 08:04:36 +02:00
Norman Maurer
c745ac0e16 [#3297] Fix race while resolve remote address that can cause NPE
Motivation:

We may produce a NPE due a race that can happen while check if a resolution was done and failed.

Modifications:

Correctly first check if the resultion is done before try to detect if it failed and so remove the race that can produce a NPE.

Result:

No more race possible while resolve address during connect.
2016-05-06 08:00:15 +02:00
Carl Mastrangelo
cf07f984b1 Add @Deprecated when the javadoc says its deprecated
Motivation:

Reduce nag warnings when compiling, make it easier for IDEs to display what's deprecated.

Modifications:

Added @Deprecated in a few places

Result:

No more warnings.
2016-05-01 20:30:13 +02:00
Scott Mitchell
04e33fd2d8 NioEventLoop closes channel even if channel is not registered
Motivation:
If a channel is deregistered from an NioEventLoop the associated SelectionKey is cancelled. If the NioEventLoop has yet to process a pending event as a result of that SelectionKey then the NioEventLoop will see the SelecitonKey is invalid and close the channel. The NioEventLoop should not close a channel if it is not registered with that NioEventLoop.

Modifications:
- NioEventLoop.processSelectedKeys should check that the channel is still registered to itself before closing the channel

Result:
NioEventLoop doesn't close a channel that is no longer registered to it when the SelectionKey is invalid
Fixes https://github.com/netty/netty/issues/5125
2016-04-21 09:17:14 -07:00
Norman Maurer
572bdfb494 [maven-release-plugin] prepare for next development iteration 2016-04-10 08:37:18 +02:00
Norman Maurer
c6121a6f49 [maven-release-plugin] prepare release netty-4.1.0.CR7 2016-04-10 08:36:56 +02:00
Norman Maurer
6e919f70f8 [maven-release-plugin] rollback the release of netty-4.1.0.CR7 2016-04-09 22:13:44 +02:00
Norman Maurer
4cdd51509a [maven-release-plugin] prepare release netty-4.1.0.CR7 2016-04-09 22:05:34 +02:00
Scott Mitchell
abce89d1bc Revert "[#5028] Fix re-entrance issue with channelWritabilityChanged(...) and write(...)"
Motivation:
Revert d0943dcd30. Delaying the notification of writability change may lead to notification being missed. This is a ABA type of concurrency problem.

Modifications:
- Revert d0943dcd30.

Result:
channelWritabilityChange will be called on every change, and will not be suppressed due to ABA scenario.
2016-04-09 20:32:47 +02:00
Scott Mitchell
a0b28d6c82 Fix potential assertion error introduced by 0bc93dd
Motivation:
Commit 0bc93dd introduced a potential assertion failure, if the deprecated method would be used.

Modifications:
Fix the potential assertion error.

Result:
Regression removed
2016-04-08 09:44:44 -07:00
Trustin Lee
32cfe25132 Fix checkstyle 2016-04-08 17:44:52 +09:00
Trustin Lee
c1a3cc32e7 Fix an assertion error introduced by 0bc93dda08
Motivation:
    
Commit 0bc93dda08 introduced an assertion
failure.

Modifications:

Fix the assertion error.

Result:

Regression removed
2016-04-08 17:23:32 +09:00
stroller
84e281ced9 fix one java doc issue: extra }
Motivation:

There is one extra } for WriteBufferWaterMark's javadoc:
{@linkplain #high}  high water mark}

The generated javadoc will show the content: "the high high water mark}"

Modifications:

remove the }

Result:
The generated javadoc will show the content: "the high water mark" instead of "the high high water mark}"
2016-04-08 09:47:26 +02:00
Scott Mitchell
0bc93dda08 NIO autoReadClear should also clear the SelectionKey
Motivation:
Prior to 5b48fc284e setting readPending to false when autoReadClear was called was enough because when/if the EventLoop woke up with a read event we would first check if readPending was true and if it is not, we would return early. Now that readPending will only be set in the EventLoop it is not necessary to check readPending at the start of the read methods, and we should instead remove the OP_READ from the SelectionKey when we also set readPending to false.

Modifications:
- autoReadCleared should call AbstractNioUnsafe.removeReadOp

Result:
NIO is now consistent with EPOLL and removes the READ operation on the selector when autoRead is cleared.
2016-04-08 00:03:37 -07:00
Scott Mitchell
5b48fc284e Make OIO/NIO/EPOLL autoReadClear consistent
Motivation:
OIO/NIO use a volatile variable to track if a read is pending. EPOLL does not use a volatile an executes a Runnable on the event loop thread to set readPending to false. These mechansims should be consistent, and not using a volatile variable is preferable because the variable is written to frequently in the event loop thread.
OIO also does not set readPending to false before each fireChannelRead operation and may result in reading more data than the user desires.

Modifications:
- OIO/NIO should not use a volatile variable for readPending
- OIO should set readPending to false before each fireChannelRead

Result:
OIO/NIO/EPOLL are more consistent w.r.t. readPending and volatile variable operations are reduced
Fixes https://github.com/netty/netty/issues/5069
2016-04-06 12:32:14 -07:00
Norman Maurer
d0943dcd30 [#5028] Fix re-entrance issue with channelWritabilityChanged(...) and write(...)
Motivation:

When always triggered fireChannelWritabilityChanged() directly when the update the pending bytes in the ChannelOutboundBuffer was made from within the EventLoop. This is problematic as this can cause some re-entrance issue if the user has a custom ChannelOutboundHandler that does multiple writes from within the write(...) method and also has a handler that will intercept the channelWritabilityChanged event and trigger another write when the Channel is writable. This can also easily happen if the user just use a MessageToMessageEncoder subclass and triggers a write from channelWritabilityChanged().

Beside this we also triggered fireChannelWritabilityChanged() too often when a user did a write from outside the EventLoop. In this case we increased the pending bytes of the outboundbuffer before scheduled the actual write and decreased again before the write then takes place. Both of this may trigger a fireChannelWritabilityChanged() event which then may be re-triggered once the actual write ends again in the ChannelOutboundBuffer.

The third gotcha was that a user may get multiple events even if the writability of the channel not changed.

Modification:

- Always invoke the fireChannelWritabilityChanged() later on the EventLoop.
- Only trigger the fireChannelWritabilityChanged() if the channel is still active and if the writability of the channel changed. No need to cause events that were already triggered without a real writability change.
- when write(...) is called from outside the EventLoop we only increase the pending bytes in the outbound buffer (so that Channel.isWritable() is updated directly) but not cause a fireChannelWritabilityChanged(). The fireChannelWritabilityChanged() is then triggered once the task is picked up by the EventLoop as usual.

Result:

No more re-entrance possible because of writes from within channelWritabilityChanged(...) method and no events without a real writability change.
2016-04-06 10:07:13 +02:00
Scott Mitchell
9fb86a380d NIO/EPOLL readPending set to false incorrectly
Motivation:
441aa4c575 introduced a bug in transport-native-epoll where readPending is set to false before a read is attempted, but this should happen before fireChannelRead is called. The NIO transport also only sets the readPending variable to false on the first read in the event loop. This means that if the user only calls read() on the first channelRead(..) the select loop will still listen for read events even if the user does not call read() on subsequent channelRead() or channelReadComplete() in the same event loop run. If the user only needs 2 channelRead() calls then by default they will may get 14 more channelRead() calls in the current event loop, and then 16 more when the event loop is woken up for a read event. This will also read data off the TCP stack and allow the peer to queue more data in the local RECV buffers.

Modifications:
- readPending should be set to false before each call to channelRead()
- make NIO readPending set to false consistent with EPOLL

Result:
NIO and EPOLL transport set readPending to false at correct times which don't read more data than intended by the user.
Fixes https://github.com/netty/netty/issues/5082
2016-04-06 00:09:49 -07:00
Norman Maurer
d602277204 Include cause that was used to notify the promise when logging an failed try to notify it.
Motivation:

When a promise is notified that was already added to the ChannelOutboundBuffer and we try to notify it later on we only see a warning that it was notified before. This is often not very useful as we have no idea where it was notified at all. We can do better in case it was failed before (which is most of the times the case) and just also log the cause that was used for it.

Modifications:

Add the cause that was used to notify the promise when we fail to notify it as part of the ChannelOutboundBuffer.

Result:

Easier to debug user errors.
2016-04-05 21:13:51 +02:00
Norman Maurer
7fb475a223 Fix typo missed in f46cfbc590 2016-04-05 15:32:54 +02:00
Norman Maurer
f46cfbc590 [#5059] Deprecate method with typo and introduce a new one without typo
Motivation:

There is a spelling error in FileRegion.transfered() as it should be transferred().

Modifications:

Deprecate old method and add a new one.

Result:

Fix typo and can remove the old method later.
2016-04-05 15:06:46 +02:00