Commit Graph

230 Commits

Author SHA1 Message Date
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
Travis Haagen
a75dcb2756 Made it easier to use custom ChannelId instances with Channel implementations that rely on the AbstractChannel(Channel parent) constructor.
Motivation:

The AbstractChannel(Channel parent) constructor was previously hard-coded to always
call DefaultChannelId.newInstance(), and this made it difficult to use a custom
ChannelId implementation with some commonly used Channel implementations.

Modifications:

Introduced newId() method in AbstractChannel, which by default returns
DefaultChannelId.newInstance() but can be overridden by subclasses. Added
ensureDefaultChannelId() test to AbstractChannelTest, to ensure the prior
behavior of calling DefaultChannelId.newInstance() still holds with the
AbstractChannel(Channel parent) constructor.

Result:

AbstractChannel now has the protected newId() method, but there is no functional
difference.
2016-02-02 08:33:42 +01:00
Norman Maurer
e969b6917c Let CombinedChannelDuplexHandler correctly handle exceptionCaught. Related to [#4528]
Motivation:

ChannelInboundHandler and ChannelOutboundHandler both can implement exceptionCaught(...) method and so we need to dispatch to both of them.

Modifications:

- Correctly first dispatch exceptionCaught to the ChannelInboundHandler but also make sure the next handler it will be dispatched to will be the ChannelOutboundHandler
- Add removeInboundHandler() and removeOutboundHandler() which allows to remove one of the combined handlers
- Let *Codec extends it and not ChannelHandlerAppender
- Remove ChannelHandlerAppender

Result:

Correctly handle events and also have same behavior as in 4.0
2016-01-18 09:54:48 +01:00
Norman Maurer
1848e73ce6 Ensure connectPromise is not notified before fireChannelActive() is called.
Motivation:

Our contract in Channels is that the promise should always be notified before the actual callbacks of the ChannelInboundHandler are called. This was not done in the LocalChannel and so the behavior was different to other Channel implementations.

Modifications:

- First complete the ChannelPromise then call fireChannelActive()
- Guard against NPE when doClose() was called before the task was executed.

Result:

Consistent behavior between LocalChannel and other Channel implementations.
2016-01-18 09:28:43 +01:00
Norman Maurer
8b123a5546 Fix AbstractChannelTest errors caused by incorrect mocking
Motivation:

In AbstractChannelTest we not correctly mocked some methods which could lead to test errors. That said it only showed up here when running from the IDE and not from the cmdLine.

Modifications:

Mock methods that are needed for the test

Result:

Test pass in the IDE as well.
2016-01-14 20:57:42 +01:00
Norman Maurer
4d854cc149 Allow to change if EmbeddedChannel should handle close() and disconnect() different.
Motivation:

At the moment EmbeddedChannel always handle close() and disconnect() the same way which also means that ChannelOutboundHandler.disconnect(...) will never called. We should allow to specify if these are handle different or not to make the use of EmbeddedChannel more flexible.

Modifications:

Add 2 other constructors which allow to specify if disconnect / close are handled the same way or differently.

Result:

More flexible usage of EmbeddedChannel possible.
2016-01-14 07:19:23 +01:00
Stephane Landelle
6393506b97 Extract SocketAdress logic from NameResolver
Motivation:

As discussed in #4529, NameResolver design shouldn't be resolving SocketAddresses (or String name + port) and return InetSocketAddresses. It should resolve String names and return InetAddresses.
This SocketAddress to InetSocketAddresses resolution is actually a different concern, used by Bootstrap.

Modifications:

Extract SocketAddress to InetSocketAddresses resolution concern to a new class hierarchy named AddressResolver.
These AddressResolvers delegate to NameResolvers.

Result:

Better separation of concerns.

Note that new AddressResolvers generate a bit more allocations because of the intermediate Promise and List<InetAddress>.
2015-12-14 14:03:50 +01:00
Scott Mitchell
641505a5d2 DefaultChannelConfig maxMessagesPerRead default not always set
Motivation:
ChannelMetadata has a field minMaxMessagesPerRead which can be confusing. There are also some cases where static instances are used and the default value for channel type is not being applied.

Modifications:
- use a default value which is set unconditionally to simplify
- make sure static instances of MaxMessagesRecvByteBufAllocator are not used if the intention is that the default maxMessagesPerRead should be derived from the channel type.

Result:
Less confusing interfaces in ChannelMetadata and ChannelConfig. Default maxMessagesPerRead is correctly applied.
2015-11-25 15:14:07 -08:00
nmittler
227e67900e Fixing spammy logging for CoalescingBufferQueueTest
Motivation:

The CoalescingBufferQueueTest is somewhat relaxed with its releasing of test buffers, using safeRelease to generically deal with tests that may or may not release the buffers. SafeRelease generates logs, however, when the release fails.

Modifications:

Tightened up the individual test methods to verify that the buffers are released properly.

Result:

Fixes #4497
2015-11-25 07:16:05 -08:00
Sergio Bossa
187efca9aa Improved DefaultChannelPipeline#destroy() to avoid spinning continuously in case of custom executors.
Motivation:

The previous DefaultChannelPipeline#destroy() implementation, introduced in #3156, is suboptimal as it can cause the for loop to continuously spin if the executor used by a given handler is unable to "recognize" the event loop.
It could be objected that it's the custom executor responsibility to properly implement the inEventLoop() method, but some implementetaions might not be able to do that for performance reasons, and even so, it's always better to be safe against API misuse, in particular when it is not possible to fail fast and the alternative is rather some sutle behaviour.

Modifications:

The patch simply avoids the recursive spin by explicitly passing the "in event loop" condition as a boolean parameter, preserving the same guarantees offered by #3156. A unit test has also been added.

Result:

All channel events are correctly called and no high CPU usage is seen anymore.
2015-11-06 19:37:53 +01:00
Norman Maurer
8687475eb7 [#4373] Fix assert error when trying to release Channel to closed FixedChannelPool
Motivation:

Once a FixedChannelPool was closed we must not allow to acquire or release Channels to prevent assert errors.

Modifications:

Fail release and acquire calls when FixedChannelPool is closed.

Result:

No more assert errors.1
2015-10-24 11:56:46 +02:00
Richard DiCroce
b58036aeea Improve flexibility of EmbeddedChannel ID
Motivation:

EmbeddedChannelId#hashCode() and equals() do not behave correctly if an
instance is serialized and then deserialized. Additionally,
EmbeddedChannel does not allow use of any other type of ChannelId, and
EmbeddedChannelId is (mostly) a singleton instance. This creates a
problem for unit tests that construct multiple EmbeddedChannels and
expect each channel to have a unique ID.

Modifications:

EmbeddedChannelId is modified so equals() will return true and
hashCode() will return the same value for any instance of the class.
EmbeddedChannel is modified to allow a ChannelId to be specified when
the channel is constructed. Tests added for both changes.

Result:

EmbeddedChannelId now behaves correctly when deserialized, and
EmbeddedChannels can now have unique IDs.
2015-10-14 10:40:02 +02:00
Norman Maurer
2d7e957a23 Cleanup PendingWriteQueueTest
Motivation:

PendingWriteQueueTest needs some cleanup.

Modifications:

- Cleanup code to remove deprecation warnings
- use static imports

Result:

No more warnings
2015-10-10 19:59:23 +02:00
Norman Maurer
fbaf5d06e6 [#4316] Ensure pending tasks are run when EmbeddedChannel.close(...) or disconnect(...) is called.
Motivation:

We missed to run all pending tasks when EmbeddedChannel.close(...) or disconnect(...) was called. Because of this channelInactive(...) / channelUnregistered(...) of the handlers were never called.

Modifications:

Correctly run all pending tasks and cancel all not ready scheduled tasks when close or disconnect was called.

Result:

Correctly run tasks on close / disconnect and have channelInactive(...) / channelUnregistered(...) called.
2015-10-07 09:28:00 +02:00
Scott Mitchell
1e763b6504 LocalChannel write when peer closed leak
Motivation:
If LocalChannel doWrite executes while the peer's state changes from CONNECTED to CLOSED it is possible that some promise's won't be completed and buffers will be leaked.

Modifications:
- Check the peer's state in doWrite to avoid a race condition

Result:
All write operations should release, and the associated promise should be completed.
2015-09-01 13:13:02 -07:00
Scott Mitchell
d3dcc7f658 LocalChannelWrite event sequencing issue
Motivation:
https://github.com/netty/netty/pull/4143 addressed a few ordering issues but an ordering issue still remained if the Promise for a write completes, and a listener of that promise does a write on a peer channel. The ordering was subject to how potentially 2 different executors would run a task, but it should be coordinated such that the first write is read first.

Modifications:
- Keep track of the finishPeerRead task run on the executor if necessary and ensure it completes before current channel read occurs

Result:
Ordering of events for echo type situations is preserved.
2015-08-28 11:26:59 -07:00
Scott Mitchell
e37069b947 LocalChannel Event Ordering Error
Motivation:
When a LocalChannel write operation occurs, the promise associated with the write operation is marked successful when it is added to the peer's queue, but before the peer has actually received the data. If the promise callback closes the channel then a race condition exists where the close event may occur before the data is delivered. We should preserve ordering of events.

Modifications:
- LocalChannel should track when a write is in progress, and if a close operation happens make sure the peer gets all pending read operations.

Result:
LocalChannel preserves order of operations.
Fixes https://github.com/netty/netty/issues/4118
2015-08-28 09:03:46 -07:00
Trustin Lee
fdfe3149ba Provide more control over DnsNameResolver.query() / Add NameResolver.resolveAll()
Related issues:
- #3971
- #3973
- #3976
- #4035

Motivation:

1. Previously, DnsNameResolver.query() retried the request query by its
own. It prevents a user from deciding when to retry or stop. It is also
impossible to get the response object whose code is not NOERROR.

2. NameResolver does not have an operation that resolves a host name
into multiple addresses, like InetAddress.getAllByName()

Modifications:

- Changes related with DnsNameResolver.query()
  - Make query() not retry
    - Move the retry logic to DnsNameResolver.resolve() instead.
  - Make query() fail the promise only when I/O error occurred or it
    failed to get a response
  - Add DnsNameResolverException and use it when query() fails so that
    the resolver can give more information about the failure
  - query() does not cache anymore.

- Changes related with NameResolver.resolveAll()
  - Add NameResolver.resolveAll()
  - Add SimpleNameResolver.doResolveAll()

- Changes related with DnsNameResolver.resolve() and resolveAll()
  - Make DnsNameResolveContext abstract so that DnsNameResolver can
    decide to get single or multiple addresses from it
  - Re-implement cache so that the cache works for resolve() and
    resolveAll()
  - Add 'traceEnabled' property to enable/disable trace information

- Miscellaneous changes
  - Use ObjectUtil.checkNotNull() wherever possible
  - Add InternetProtocolFamily.addressType() to remove repetitive
    switch-case blocks in DnsNameResolver(Context)
  - Do not raise an exception when decoding a truncated DNS response

Result:

- Full control over query()
- A user can now retrieve all addresses via (Dns)NameResolver.resolveAll()
- DNS cache works only for resolve() and resolveAll() now.
2015-08-18 17:40:13 +09:00
Trustin Lee
fd9ca8bbb3 Fix compilation errors 2015-08-18 12:42:33 +09:00
Ivan Bahdanau
08b73bf914 Unhealthy channel is not offered back to the pool.
Motivation:
When releasing unhealthy channel back to a pool we don't have to offer it since on acquire it will be discarded anyways.
 Also checking healthiness at release is a good idea so we don't end up having tons of unhealthy channels in the pool(unless they became unhealthy after being offered)

Modifications:
private SimpleChannelPool.offerIfHealthy() method added that is called from SimpleChannelPool.doReleaseChannel(). SimpleChannelPool.offerIfHealthy() offers channel back to pool only if channel is healthy.
Otherwise it throws setFailure exception to the promise.

 Result:
The pool is now much cleaner and not spammed with unhealthy channels.

Added ability to choose if channel health has to be validated on release by passing boolean flag.

Motivation:
Depending on performance preferences and individual use cases sometimes we would like to be able force health check of a channel at release time and do not offer it back to the pool. Other times we would want to just release channel and offer it back to the pool and check health only when we try to acquire that channel from the pool. See more details here: https://github.com/netty/netty/issues/4077#issuecomment-130461684

Modifications:
Future<Void> release(Channel channel, Promise<Void> promise, boolean offerHealthyOnly);
The offerHealthyOnly boolean flag allows developers to choose whether to do channel validation before offering it back to pool or not.
Appropriate modifications made to hierarchy of implementations of ChannelPool. offerHealthyOnly=true will force channel health to be checked before offering back to pool. offerHealthyOnly=false  will ignore channel health check and will just try just offer it back to the pool
 offerHealthyOnly=true by default.

Result:
Channel health check before offer back to pool is controlled by a flag now.

Code changed to satisfy checkstyle requirements.

Motivation:
Code needs to satisfy checkstyle requirements.

Modifications:
 SimpleChannelPool.java:279 line split to be less then 120 characters.
 SimpleChannelPool.java:280:31 space added after '{'
 SimpleChannelPool.java:282:17 space added after '{'
 SimpleChannelPoolTest.java:198 - extra white space line removed.

Result:
Code satisfies checkstyle requirements.

 offerHealthyOnly is passed as a constructor parameter now.

Motivation:
Instead of passing offerHealthyOnly as a method parameter it is better to pass it in as SimpleChannelPool or FixedChannelPool constructor.

Modifications:
 Redundant release method that takes offerHealthyOnly removed from ChannelPool.
 offerHealthyOnly parameter added to constructor for FixedChannelPool and SimpleChannelPool.

Result:
SimpleChannelPool and FixedChannelPool are now take offerHealthyOnly as a constructor parameter. Default behavior is: offerHealthyOnly=true.

Code changed to satisfy checkstyle requirements.

Motivation:
Code needs to satisfy checkstyle requirements.

Modifications:
 SimpleChannelPool.java:84: line made to be no longer then 120 characters.
 SimpleChannelPool.java:237: extra white space line removed.

Result:
Code satisfies checkstyle requirements.

Tests do not need to be too  copled to the code. Exception message should not be validated

Motivation:
We don't need our tests to be too coupled to the code. Exception type validation in tests is just good enough.

Modifications:
Exception validation message removed from SimpleChannelPoolTest.testUnhealthyChannelIsNotOffered() test.

Result:
The SimpleChannelPoolTest test is less coupled to the code now.

Stack trace set to empty for UNHEALTHY_NON_OFFERED_TO_POOL.

Motivation:
We don't need stack trace for UNHEALTHY_NON_OFFERED_TO_POOL.

Modifications:
Added  UNHEALTHY_NON_OFFERED_TO_POOL.setStackTrace(EmptyArrays.EMPTY_STACK_TRACE) to static init block.

Result:
UNHEALTHY_NON_OFFERED_TO_POOL's stack trace set to empty.

Minor code re-factorings.

Motivation:
For better code readability we need to apply several minor code re-factorings.

Modifications:
javadocs true -> {@code true}
offerHealthyOnly variable name changed to releaseHeathCheck
<p/> -> <p> in javadocs
offerHealthyOnly removed from doReleaseChannel as it not needed there.

Result:
Code quality is improved.

Code changed to satisfy checkstyle requirements.

Motivation:
Code needs to satisfy checkstyle requirements.

Modifications:
SimpleChannelPool.java:87: line made to be no longer then 120 characters.

Result:
Code satisfies checkstyle requirements.

Pull request needs to contain only necessary changes

Motivation:
The pull request should not contain unnecessary changes that are not needed as part of required functionality of pull request.

Modifications:
private void doReleaseChannel(final Channel channel, final Promise<Void> promise) - >  private void doReleaseChannel(Channel channel, Promise<Void> promise)

Result:
Pull request contains less unnecessary modifications.
2015-08-17 15:43:08 -07:00
Norman Maurer
9d417c1626 Fixing compile error, introduce by 5d011b8895 2015-08-12 14:25:45 +02:00
Ivan Bahdanau
5d011b8895 Improve the logic around acquire channel function is improved.
Motivation:
The acquire channel function resulted in calling itself several times in case when channel polled from the pool queue was unhealthy, which resulted FixedChannelPool to be called several times which in it's turn caused FixedChannelPool.acquire() to be called and resulted into acquireChannelCount to be unnecessary increased.
Example use case:
1) Create FixedChannelPool instance with one channel in the pool: new FixedChannelPool(cb, handler, 1)
2) Acquire channel A from the pool
3) close the channel A
4) Return it back to the pool
5) Acquire channel from the same pool again
Expected result:
new channel created and acquired, channel A that has been closed discarded and removed from the pool from being unhealthy
Actual result:
Channel A had been removed from the pool, how ever the new channel had never be acquired, instead the request to acquire had been added to the pending queue in FixedChannelPool and the acquireChannelCount is increased by one. The reason is that at the time when SimpleChannelPool figured out that the channel was unhealthy called FixedChannelPool.acquire to try to acquire new channel, how ever the request was added to the pendingTakQueue because by the time when FixedChannelPool.acquire was called, the acquireChannelCount was already "1" so new channel ould not be created cause of maxChannelsLimit=1.

Modifications:
The suggested approach modifies the SimpleChannelPool in a way so that when channel detected to be unhealthy it calls private method SimpleChannelPool.acquireHealthyFromPoolOrNew() which guarantees that SimpleChannelPool actually either finds a healthy channel in the pool and returns it or causes the promise.cause() in case when new channel was failed to be created.

 Result:
The  ```acquiredChannelCount``` is now calculated correctly as a result of SimpleChannelPool.acquire() of not being recursive on overridable acquire method.
2015-08-12 06:35:56 +02:00
Norman Maurer
f1c5f0e0c7 [#3967] Guard against NPE in PendingWriteQueue
Motivation:

If the Channel is already closed when the PendingWriteQueue is created it will generate a NPE when add or remove is called later.

Modifications:

Add null checks to guard against NPE.

Result:

No more NPE possible.
2015-07-17 21:31:27 +02:00
Louis Ryan
e6ed3ce6b1 Make CoalescingBufferQueueTest properly release all buffers including the composites removed from the queue.
Motivation:
Test was leaving composite buffers taken from the queue unreleased.

Modifications:
Make the test release buffers.

Result:
Nagging about leaked buffers should stop.
2015-07-16 21:46:45 +02:00
Louis Ryan
cdbf43596d Add generic utility for enqueuing buffers with promises and dequeueing them in arbitrary byte ranges.
Motivation:
Simplifies writing code that needs to merge or slice a sequence of buffer & promise pairs into chunks of arbitrary sizes.
For example in HTTP2 we merge or split buffers across fixed-size DATA frame boundaries.

Modifications:
Add new utility class CoalescingBufferQueue

Result:
Following this change HTTP2 code will switch to use it instead of CompositeByteBuffer for DATA frame coalescing.
2015-07-09 07:50:02 -07:00
Norman Maurer
8650679f29 Fix regression introduced by 11f9e9084b
Motivation:

While cherry-picked 11f9e9084b I changed the EmbeddedChannel implementation to not allow no ChannelHandlers when constructing it.
This was done by mistake.

Modifications:

Revert change and add unit test.

Result:

Restore old behavior.
2015-07-08 21:37:58 +02:00
Norman Maurer
0f6559d188 [#3780] Handle ChannelInitializer exception in exceptionCaught()
Motivation:

At the moment we directly closed the Channel when an exception accoured durring initChannel(...) without giving the user any way to do extra or special handling.

Modifications:

Handle the exception in exceptionCaught(...) of the ChannelInitializer which will by default log and close the Channel. This way the user can override this.

Result:

More felixible handling of exceptions.
2015-07-07 09:13:22 +02:00
Norman Maurer
11f9e9084b [#3921] EmbeddedChannel should add ChannelHandlers once registered
Motivation:

Currently in EmbeddedChannel we add the ChannelHandlers before the Channel is registered which leads to have the handlerAdded(...) callback
be called from outside the EventLoop and also prevent the user to obtain a reference to the EventLoop in the callback itself.

Modifications:

Delay adding ChannelHandlers until EmbeddedChannel is registered.

Result:

Correctly call handlerAdded(...) after EmbeddedChannel is registered.
2015-07-07 08:54:27 +02:00
Norman Maurer
dd6ac55fa0 ServerBootstrap.handler(...) will add handler before Channel is registered.
Motivation:

If you set a ChannelHandler via ServerBootstrap.handler(...) it is added to the ChannelPipeline before the Channel is registered. This will lead to and IllegalStateException if a user tries to access the EventLoop in the ChannelHandler.handlerAdded(...) method.

Modifications:

Delay the adding of the ChannelHandler until the Channel was registered.

Result:

No more IllegalStateException.
2015-07-07 08:44:13 +02:00
Trustin Lee
5f59591a72 Fix sporadic assertion failure in SingleThreadEventLoopTest
Motivation:

SingleThreadEventLoopTest.testScheduleTaskAtFixedRate() fails often due to:

- too little tolerance
- incorrect assertion (it compares only with the previous timestamp)

Modifications:

- Increase the timestamp difference tolerance from 10ms to 20ms
- Improve the timestamp assertion so that the comparison is performed against the first recorded timestamp
- Misc: Fix broken Javadoc tag

Result:

More build stability
2015-06-01 14:45:36 +09:00
Norman Maurer
56c98839c3 [#3218] Add ChannelPool / ChannelPoolMap abstraction and implementations
Motivation:

Many projects need some kind a Channel/Connection pool implementation. While the protocols are different many things can be shared, so we should provide a generic API and implementation.

Modifications:

Add ChannelPool / ChannelPoolMap API and implementations.

Result:

Reusable / Generic pool implementation that users can use.
2015-04-30 12:13:19 +02:00
Norman Maurer
ccde870b38 Revert "Ensure channelReadComplete() is called only when necessary"
This reverts commit 27a25e29f7.
2015-04-20 09:10:41 +02:00
Norman Maurer
a123c495e1 Revert "Add another test case for channelReadComplete() suppression"
This reverts commit daa04cb4f1.
2015-04-20 09:10:14 +02:00
Norman Maurer
a709553819 Allow to use EmbeddedChannel.schedule*(...)
Motivation:

At the moment when EmbeddedChannel is used and a ChannelHandler tries to schedule and task it will throw an UnsupportedOperationException. This makes it impossible to test these handlers or even reuse them with EmbeddedChannel.

Modifications:

- Factor out reusable scheduling code into AbstractSchedulingEventExecutor
- Let EmbeddedEventLoop and SingleThreadEventExecutor extend AbstractSchedulingEventExecutor
- add EmbbededChannel.runScheduledPendingTasks() which allows to run all scheduled tasks that are ready

Result:

Embeddedchannel is now usable even with ChannelHandler that try to schedule tasks.
2015-02-20 11:57:32 +01:00
Norman Maurer
99bd43ed51 Allow to get existing ChannelOption / AttributeKey from String
Motivation:

We should allow to get a ChannelOption/AttributeKey from a String. This will make it a lot easier to make use of configuration files in applications.

Modifications:

- Add exists(...), newInstance(...) method to ChannelOption and AttributeKey and alter valueOf(...) to return an existing instance for a String or create one.
- Add unit tests.

Result:

Much more flexible usage of ChannelOption and AttributeKey.
2015-02-18 09:29:37 +01:00
Trustin Lee
daa04cb4f1 Add another test case for channelReadComplete() suppression
Related commit:
- a41b46ff43

Motivation:

We need a test case for the commit above.
2015-02-09 16:29:21 +09:00
Trustin Lee
27a25e29f7 Ensure channelReadComplete() is called only when necessary
Motivation:

Even if a handler called ctx.fireChannelReadComplete(), the next handler
should not get its channelReadComplete() invoked if fireChannelRead()
was not invoked before.

Modifications:

- Ensure channelReadComplete() is invoked only when the handler of the
  current context actually produced a message, because otherwise there's
  no point of triggering channelReadComplete().
  i.e. channelReadComplete() must follow channelRead().
- Fix a bug where ctx.read() was not called if the handler of the
  current context did not produce any message, making the connection
  stall. Read the new comment for more information.

Result:

- channelReadComplete() is invoked only when it makes sense.
- No stale connection
2015-02-07 16:13:56 +09:00
Norman Maurer
abf7afca76 [#3367] Fix re-entrance bug in PendingWriteQueue
Motivation:

Because of a re-entrance bug in PendingWriteQueue it was possible to get the queue corrupted and also trigger an IllegalStateException caused by multiple recycling of the internal PendingWrite objects.

Modifications:

- Correctly guard against re-entrance

Result:

No more IllegalStateException possible
2015-02-06 19:49:34 +01:00
Trustin Lee
298e7af647 Trigger channelWritabilityChanged() later to avoid reentrance
Related: #3212

Motivation:

When SslHandler and ChunkedWriteHandler exists in a pipeline together,
it is possible that ChunkedWriteHandler.channelWritabilityChanged()
invokes SslHandler.flush() and vice versa. Because they can feed each
other (i.e. ChunkedWriteHandler.channelWritabilityChanged() ->
SslHandler.flush() -> ChunkedWriteHandler.channelWritabilityChanged() ->
..), they can fall into an inconsistent state due to reentrance (e.g.
bad MAC record at the remote peer due to incorrect ordering.)

Modifications:

- Trigger channelWritabilityChanged() using EventLoop.execute() when
  there's a chance where channelWritabilityChanged() can cause a
  reentrance issue
- Fix test failures caused by the modification

Result:

Fix the handler reentrance issues related with a
channelWritabilityChanged() event
2014-12-10 18:40:26 +09:00
Trustin Lee
3957a88a94 Make PendingWriteQueue.recycle() update its state before triggering an event
Related: #3212

Motivation:

PendingWriteQueue.recycle() updates its data structure after triggering
a channelWritabilityChanged() event. It causes a rare corruption such as
double free when channelWritabilityChanged() method accesses the
PendingWriteQueue.

Modifications:

Update the state of PendingWriteQueue before triggering an event.

Result:

Fix a rare double-free problem
2014-12-07 23:27:33 +09:00
Frank Barber
f4d3f81d6c Prevent channel re-registration from firing channelActive
Motivation:

AbstractUnsafe considers two possibilities during channel registration. First,
the channel may be an outgoing connection, in which case it will be registered
before becoming active. Second, the channel may be an incoming connection in,
which case the channel will already be active when it is registered. To handle
the second case, AbstractUnsafe checks if the channel is active after
registration and calls ChannelPipeline.fireChannelActive() if so.  However, if
an active channel is deregistered and then re-registered this logic causes a
second fireChannelActive() to be invoked. This is unexpected; it is reasonable
for handlers to assume that this method will only be invoked once per channel.

Modifications:

This change introduces a flag into AbstractUnsafe to recognize if this is the
first or a subsequent registration. ChannelPipeline.fireChannelActive() is only
possible for the first registration.

Result:

ChannelPipeline.fireChannelActive() is only called once.
2014-11-30 19:51:30 +01:00
Trustin Lee
9826d9bc1a Fix compilation errors in ChannelOutboundBufferTest 2014-10-25 16:57:22 +09:00
Trustin Lee
d59629377c Implement user-defined writability flags
Related: #2945

Motivation:

Some special handlers such as TrafficShapingHandler need to override the
writability of a Channel to throttle the outbound traffic.

Modifications:

Add a new indexed property called 'user-defined writability flag' to
ChannelOutboundBuffer so that a handler can override the writability of
a Channel easily.

Result:

A handler can override the writability of a Channel using an unsafe API.
For example:

  Channel ch = ...;
  ch.unsafe().outboundBuffer().setUserDefinedWritability(1, false);
2014-10-25 15:59:13 +09:00
Trustin Lee
e848066cab Name resolver API and DNS-based name resolver
Motivation:

So far, we relied on the domain name resolution mechanism provided by
JDK.  It served its purpose very well, but had the following
shortcomings:

- Domain name resolution is performed in a blocking manner.
  This becomes a problem when a user has to connect to thousands of
  different hosts. e.g. web crawlers
- It is impossible to employ an alternative cache/retry policy.
  e.g. lower/upper bound in TTL, round-robin
- It is impossible to employ an alternative name resolution mechanism.
  e.g. Zookeeper-based name resolver

Modification:

- Add the resolver API in the new module: netty-resolver
- Implement the DNS-based resolver: netty-resolver-dns
  .. which uses netty-codec-dns
- Make ChannelFactory reusable because it's now used by
  io.netty.bootstrap, io.netty.resolver.dns, and potentially by other
  modules in the future
  - Move ChannelFactory from io.netty.bootstrap to io.netty.channel
  - Deprecate the old ChannelFactory
  - Add ReflectiveChannelFactory

Result:

It is trivial to resolve a large number of domain names asynchronously.
2014-10-16 17:05:20 +09:00
Norman Maurer
246b65c6b6 [#2586] Use correct EventExecutor to notify for bind failures on late registration
Motivation:

We used the wrong EventExecutor to notify for bind failures if a late registration was done.

Modifications:

Use the correct EventExecutor to notify and only use the GlobelEventExecutor if the registration fails itself.

Result:

The correct Thread will do the notification.
2014-08-20 16:34:57 +02:00
Norman Maurer
9b99ac0bcb [#2771] Correctly handle constructing of EmbeddedChannel
Motivation:

Because of an incorrect logic in teh EmbeddedChannel constructor it is not possible to use EmbeddedChannel with a ChannelInitializer as constructor argument. This is because it adds the internal LastInboundHandler to its ChannelPipeline before it register itself to the EventLoop.

Modifications:

First register self to EventLoop before add LastInboundHandler to the ChannelPipeline.

Result:

It's now possible to use EmbeddedChannel with ChannelInitializer.
2014-08-15 12:21:37 +02:00
Norman Maurer
02e7e53cbb [#2752] Add PendingWriteQueue for queue up writes
Motivation:

Sometimes ChannelHandler need to queue writes to some point and then process these. We currently have no datastructure for this so the user will use an Queue or something like this. The problem is with this Channel.isWritable() will not work as expected and so the user risk to write to fast. That's exactly what happened in our SslHandler. For this purpose we need to add a special datastructure which will also take care of update the Channel and so be sure that Channel.isWritable() works as expected.

Modifications:

- Add PendingWriteQueue which can be used for this purpose
- Make use of PendingWriteQueue in SslHandler

Result:

It is now possible to queue writes in a ChannelHandler and still have Channel.isWritable() working as expected. This also fixes #2752.
2014-08-12 06:38:22 +02:00
Norman Maurer
869687bd71 Port ChannelOutboundBuffer and related changes from 4.0
Motivation:

We did various changes related to the ChannelOutboundBuffer in 4.0 branch. This commit port all of them over and so make sure our branches are synced in terms of these changes.

Related to [#2734], [#2709], [#2729], [#2710] and [#2693] .

Modification:
Port all changes that was done on the ChannelOutboundBuffer.

This includes the port of the following commits:
 - 73dfd7c01b
 - 997d8c32d2
 - e282e504f1
 - 5e5d1a58fd
 - 8ee3575e72
 - d6f0d12a86
 - 16e50765d1
 - 3f3e66c31a

Result:
 - Less memory usage by ChannelOutboundBuffer
 - Same code as in 4.0 branch
 - Make it possible to use ChannelOutboundBuffer with Channel implementation that not extends AbstractChannel
2014-08-05 15:00:45 +02:00
Norman Maurer
750eed1804 Fix broken test after change the maximal value of the pid. Part of [#2706] 2014-07-28 10:39:44 -07:00
Trustin Lee
830091c260 Reduce the default initial capacity of ChannelOutboundBuffer
Motivation:

ChannelOutboundBuffer is basically a circular array queue of its entry
objects.  Once an entry is created in the array, it is never nulled out
to reduce the allocation cost.

However, because it is a circular queue, the array almost always ends up
with as many entry instances as the size of the array, regardless of the
number of pending writes.

At worst case, a channel might have only 1 pending writes at maximum
while creating 32 entry objects, where 32 is the initial capacity of the
array.

Modifications:

- Reduce the initial capacity of the circular array queue to 4.
- Make the initial capacity of the circular array queue configurable

Result:

We spend 4 times less memory for entry objects under certain
circumstances.
2014-07-22 13:32:25 -07:00