Commit Graph

1601 Commits

Author SHA1 Message Date
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
Norman Maurer
ea94336689 DefaultChannelHandlerInvoker should work with non AbstractChannelHandlerContext sub-classes.
Motivation:

DefaultChannelHandlerInvoker currently blindly cast to AbstractChannelHandlerContext without checking if the ChannelHandlerContext is really a sub-type of it. It should check it first and if not just use slow-path implementation.

Modifications:

Do instanceof check first and if it fails just create a new Runnable instance of used the cached.

Result:

DefaultChannelHandlerInvoker works with any ChannelHandlerContext implementations.
2016-04-05 13:21:07 +02:00
Trustin Lee
3b941c2a7c [maven-release-plugin] prepare for next development iteration 2016-04-02 01:25:05 -04:00
Trustin Lee
7368ccc539 [maven-release-plugin] prepare release netty-4.1.0.CR6 2016-04-02 01:24:55 -04:00
jiafu1115
3e5dcb5f3e [#3806] Setting WRITE_BUFFER_LOW_WATER_MARK before WRITE_BUFFER_HIGH_WATER_MARK results in an internal Exception
Motivation:

Setting the WRITE_BUFFER_LOW_WATER_MARK before WRITE_BUFFER_HIGH_WATER_MARK results in an internal Exception (appears only in the logs) if the value is larger than the default high water mark value. The WRITE_BUFFER_HIGH_WATER_MARK call appears to have no effect in this context.

Setting the values in the reverse order works.

Modifications:

- deprecated ChannelOption.WRITE_BUFFER_HIGH_WATER_MARK and
ChannelOption.WRITE_BUFFER_LOW_WATER_MARK.
- add one new option called ChannelOption.WRITE_BUFFER_WATER_MARK.

Result:
The high/low water mark values limits caused by default values are removed.

Setting the WRITE_BUFFER_LOW_WATER_MARK before WRITE_BUFFER_HIGH_WATER_MARK results in an internal Exception (appears only in the logs) if the value is larger than the default high water mark value. The WRITE_BUFFER_HIGH_WATER_MARK call appears to have no effect in this context.

Setting the values in the reverse order works.
2016-03-31 13:44:44 +02:00
Tibor Csögör
9d4fae308c ChannelInitializer: change propagation of channelRegistered event
Motivation:

If a handler is added to the pipeline within ChannelInitializer::initChannel via
addFirst(...) then it will not receive the channelRegistered event.  The same
handler added via addLast(...) will receive the event.  This different behavior
is unlikely to be expected by users and can cause confusion.

Modifications:

Let ChannelInitializer::channelRegistered propagate the event by passing it to
the pipeline instead of firing it on the ChannelHandlerContext.

Result:

The channelRegistered event is propagated to handlers regardless of the method
used to add it to the pipeline (addFirst/addLast).
2016-03-31 09:01:00 +02:00
Scott Mitchell
0c839d9e0a EPOLL SelectStrategy
Motivation:
NIO now supports a pluggable select strategy, but EPOLL currently doesn't support this. We should strive for feature parity for EPOLL.

Modifications:
- Add SelectStrategy to EPOLL transport.

Result:
EPOLL transport supports SelectStategy.
2016-03-30 15:11:35 -07:00
Michael Nitschinger
5d76daf33b Allow to customize NIO (channel) select strategies.
Motivation:

Under high throughput/low latency workloads, selector wakeups are
degrading performance when the incoming operations are triggered
from outside of the event loop. This is a common scenario for
"client" applications where the originating input is coming from
application threads rather from the socket attached inside the
event loops.

As a result, it can be desirable to defer the blocking select
so that incoming tasks (write/flush) do not need to wakeup
the selector.

Modifications:

This changeset adds the notion of a generic SelectStrategy which,
based on its contract, allows the implementation to optionally
defer the blocking select based on some custom criteria.

The default implementation resembles the original behaviour, that
is if tasks are in the queue `selectNow()` and move on, and if no
tasks need to be processed go into the blocking select and wait
for wakeup.

The strategy can be customized per `NioEventLoopGroup` in the
constructor.

Result:

High performance client applications are now given the chance to
customize for how long the actual selector blocking should be
deferred by employing a custom select strategy.
2016-03-30 15:01:25 -07:00
Norman Maurer
2facb7affd Change DefaultChannelId visibility to default. Related to [#5053]
Motivation:

There is no need to make DefaultChannelId package private as it may be useful for the user. For example EmbeddedChannel allows to inject a ChannelId when it is constructed. For this case the user can just use DefaultChannelId.

Modifications:

Change visibility of DefaultChannelId to public.

Result:

It's possible to create a new instance of DefaultChannelId by the user.
2016-03-30 17:39:32 +02:00
Norman Maurer
cee38ed2b6 [maven-release-plugin] prepare for next development iteration 2016-03-29 16:45:13 +02:00
Norman Maurer
9cd9e7daeb [maven-release-plugin] prepare release netty-4.1.0.CR5 2016-03-29 16:44:33 +02:00
Norman Maurer
86b9656167 Correctly run pending tasks before flush and also remove incorrect assert.
Motivation:

We need to ensure we run all pending tasks before doing any flush in writeOutbound(...) to ensure all pending tasks are run first. Also we should remove the assert of the future and just add a listener to it so it is processed later if needed. This is true as a user may schedule a write for later execution.

Modifications:

- Remove assert of future in writeOutbound(...)
- Correctly run pending tasks before doing the flush and also before doing the close of the channel.
- Add unit tests to proof the defect is fixed.

Result:

Correclty handle the situation of delayed writes.
2016-03-29 14:30:23 +02:00
Scott Mitchell
99c85ef4f5 cf171ff525 Close Regression
Motivation:
cf171ff525 introduced a change in behavior when dealing with closing channel in the read loop. This changed behavior may use stale state to determine if a channel should be shutdown and may be incorrect.

Modifications:
- Revert the usage of potentially stale state

Result:
Closing a channel in the read loop is based upon current state instead of potentially stale state.
2016-03-24 14:52:04 -07:00
Norman Maurer
9a183ec38f Add methods to easily release messages from inbound / outbound buffer of EmbeddedChannel
Motivation:

Often the user uses EmbeddedChannel within unit tests where the only "important" thing is to know if any pending messages were in the buffer and then release these.
We should provide methods for this so the user not need to manually loop through these and release.

Modifications:

Add methods to easily handle releasing of messages.

Result:

Less boiler-plate code for the user to write.
2016-03-24 11:03:30 +01:00
Norman Maurer
28d03adbfe [maven-release-plugin] prepare for next development iteration 2016-03-21 11:51:50 +01:00
Norman Maurer
4653dc1d05 [maven-release-plugin] prepare release netty-4.1.0.CR4 2016-03-21 11:51:12 +01:00
Scott Mitchell
fc099292fd HTTP/2 DefaultHttp2ConnectionEncoder data frame size incorrect if error
Motivation:
If an error occurs during a write operation then DefaultHttp2ConnectionEncoder.FlowControlledData will clear the CoalescingBufferQueue which will reset the queue's readable bytes to 0. To recover from an error the DefaultHttp2RemoteFlowController will attempt to return bytes to the flow control window, but since the frame has reset its own size this will lead to invalid flow control accounting.

Modifications:
- DefaultHttp2ConnectionEncoder.FlowControlledData should not reset its size if an error occurs

Result:
No more flow controller errors due to DefaultHttp2ConnectionEncoder.FlowControlledData setting its size to 0 if an error occurs.
2016-03-18 11:38:01 -07:00
Tibor Csögör
6e840d8e62 trivial javadoc fixes
- fix the formatting of the diagram in ChannelFuture's javadoc
- update external link in AutobahnServer
- fix various spelling issues
2016-03-16 20:18:29 +01:00
Scott Mitchell
45849b2fa8 Deprecate PromiseAggregator
Motivation:
PromiseAggregator's API allows for the aggregate promise to complete before the user is done adding promises. In order to support this use case the API structure would need to change in a breaking manner.

Modifications:
- Deprecate PromiseAggregator and subclasses
- Introduce PromiseCombiner which corrects these issues

Result:
PromiseCombiner corrects the deficiencies in PromiseAggregator.
2016-03-14 10:53:30 -07:00
Max Ng
e7ee6abd70 Guard against re-entrance in PendingWriteQueue.
Motivation:

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

Modifications:

removeAndFailAll() should repeat until the queue is finally empty.

Result:

assertEmpty() will always hold.
2016-03-14 08:52:43 +01:00
Norman Maurer
45d291bb15 Add asserts so users will see errors when try to use methods from outside the EventLoop.
Motivation:

We should guard users from using Unsafe methods from outside the EventLoop if not designed to do so.

Modifications:

Add asserts

Result:

Easier for users to detect miss-use.
2016-03-08 14:59:43 +01:00
Scott Mitchell
d0f7f98d22 e2f5012 unit test cleanup
Motivation:
e2f5012 added unit tests which did not verify the buffer was released as it was intended to.

Modification:
- Unit tests must verify release is called

Result:
Unit tests enforce that ByteBufs are released.
2016-03-07 09:33:45 -08:00
Scott Mitchell
e2f5012f3b DefaultChannelHandlerInvoker write leak
Motivation:
DefaultChannelHandlerInvoker.invokeWrite calls a utility method validatePromise which may throw if the arguments are not valid. If this method throws then the message will not be released.

Modifications:
- If an exception is thrown the message should be released

Result:
No more leak in DefaultChannelHandlerInvoker.invokeWrite
2016-03-07 09:16:12 -08:00
Xiaoyan Lin
c7aadb5469 Fork SpscLinkedQueue and SpscLinkedAtomicQueue from JCTools
Motivation:

See #3746.

Modifications:

Fork SpscLinkedQueue and SpscLinkedAtomicQueue from JCTools based on 7846450e28

Result:

Add SpscLinkedQueue and SpscLinkedAtomicQueue and apply it in LocalChannel.
2016-02-22 15:55:52 +01:00
Norman Maurer
ca443e42e0 [maven-release-plugin] prepare for next development iteration 2016-02-19 23:00:11 +01:00
Norman Maurer
f39eb9a6b2 [maven-release-plugin] prepare release netty-4.1.0.CR3 2016-02-19 22:59:52 +01:00
Roman Timushev
23f7fc67a4 Enable shutdownOutput for EpollDomainSocketChannel 2016-02-18 18:05:51 -08:00
Norman Maurer
c6a3729e4c Ensure handlerAdded(...) and handlerRemoved(...) is always called from the right thread
Motiviation:

We should ensure that handlerAdded(...) and handlerRemoved(...) is always called from the EventExecutor that also invokes the other methods of the ChannelHandler. Also we need to ensure we always call handlerAdded(...) before any other method can be calld to ensure correct ordering.

Motifications:

- Ensure that the right thread is used to call the methods
- Ensure correct ordering
- Add tests

Result:

Respect the thread-model for handlerAdded(...) and handlerRemoved(...) and preserve correct ordering in all cases.
2016-02-12 11:18:40 -08:00
Fabian Lange
a51e2c8769 Expose Helper to obtain the "best" mac address.
Motivation:

The implementation of obtaining the best possible mac address is very good. There are many sub-par implementations proposed on stackoverflow.
While not strictly a netty concern, it would be nice to offer this util also to netty users.

Modifications:

extract DefaultChannelId#defaultMachineId code obtaining the "best" mac into a new helper called MacAddress, keep the random bytes fallback in DefaultChannelID.

Result:

New helper available.
2016-02-05 09:27:43 +01:00
Norman Maurer
75a2ddd61c [maven-release-plugin] prepare for next development iteration 2016-02-04 16:51:44 +01:00
Norman Maurer
7eb3a60dba [maven-release-plugin] prepare release netty-4.1.0.CR2 2016-02-04 16:37:06 +01:00
Norman Maurer
465a190c3f [#4805] Respect isAutoRead() once re-register Channel
Motivation:

When a channel was registered before and is re-registered we need to respect ChannelConfig.isAutoRead() and so start reading one the registration task completes. This was done "by luck" before 15162202fb.

Modifications:

Explicit start reading once a Channel was re-registered if isAutoRead() is true.

Result:

Correctly receive data after re-registration completes.
2016-02-04 15:34:24 +01:00
Norman Maurer
08a7ca3747 Correctly pass ChannelPromise on to the next ChannelOutboundHandler when use CombinedChannelDuplexHandler.
Motivation:

Due a regression introduced by e969b6917c we missed to pass the original ChannelPromise to the next ChannelOutboundHandler and so
may never notify the origin ChannelPromise. This is related to #4805.

Modifications:

- Correctly pass the ChannelPromise
- Add unit test.

Result:

Correctly pass the ChannelPromise on deregister(...)
2016-02-04 15:28:46 +01:00
fu.jian
a06708f81b fix the issue netty#2944 in 4.1
Motivation:

fix the issue netty#2944

Modifications:

use - instead of =>, use ! instead of :> due to the connection is bidirectional. What's more, toString() method don't know the direction or there is no need to know the direction when only log channel information.
add L: before local address and R: before remote address.

Result:

after the fix, log won't confuse the user
2016-02-02 21:43:38 +01:00