Commit Graph

2000 Commits

Author SHA1 Message Date
Nick Hill
a18c57ea87 Externalize lazy execution semantic for EventExecutors (#9587)
Motivation

This is already done internally for various reasons but it would make
sense i.m.o. as a top level concept: submitting a task to be run on the
event loop which doesn't need to run immediately but must still be
executed in FIFO order relative all other submitted tasks (be those
"lazy" or otherwise).

It's nice to separate this abstract "relaxed" semantic from concrete
implementations - the simplest is to just delegate to existing execute,
but for the main EL impls translates to whether a wakeup is required
after enqueuing.

Having a "global" abstraction also allows for simplification of our
internal use - for example encapsulating more of the common scheduled
future logic within AbstractScheduledEventExecutor.

Modifications

- Introduce public LazyRunnable interface and
AbstractEventExecutor#lazyExecute method (would be nice for this to be
added to EventExecutor interface in netty 5)
- Tweak existing SingleThreadEventExecutor mechanics to support these
- Replace internal use of NonWakeupRunnable (such as for pre-flush
channel writes)
- Uplift scheduling-related hooks into AbstractScheduledEventExecutor,
eliminating intermediate executeScheduledRunnable method

Result

Simpler code, cleaner and more useful/flexible abstractions - cleaner in
that they fully communicate the intent in a more general way, without
implying/exposing/restricting implementation details
2019-10-31 10:01:53 +01:00
SimpleYoung
82376fd889 Remove an unnecessary expression in NioEventLoop (#9729)
Motivation:

Remove an unnecessary expression to make the code more readable

Modification:

Remove `eventLoop == null`

Result:

Fixes #9725
2019-10-30 12:24:03 +01:00
Norman Maurer
6c061abc49
Hide Recycler implemention to allow experimenting with different implementions of an Object pool (#9715)
Motivation:

At the moment we directly extend the Recycler base class in our code which makes it hard to experiment with different Object pool implementation. It would be nice to be able to switch from one to another by using a system property in the future. This would also allow to more easily test things like https://github.com/netty/netty/pull/8052.

Modifications:

- Introduce ObjectPool class with static method that we now use internally to obtain an ObjectPool implementation.
- Wrap the Recycler into an ObjectPool and return it for now

Result:

Preparation for different ObjectPool implementations
2019-10-26 00:34:30 -07:00
root
844b82b986 [maven-release-plugin] prepare for next development iteration 2019-10-24 12:57:00 +00:00
root
d066f163d7 [maven-release-plugin] prepare release netty-4.1.43.Final 2019-10-24 12:56:30 +00:00
Norman Maurer
6c05d16967
Use @SuppressJava6Requirement for animal sniffer plugin to ensure we always guard correctly (#9655)
Motivation:

We can use the `@SuppressJava6Requirement` annotation to be more precise about when we use Java6+ APIs. This helps us to ensure we always protect these places.

Modifications:

Make use of `@SuppressJava6Requirement` explicit

Result:

Fixes https://github.com/netty/netty/issues/2509.
2019-10-14 15:54:49 +02:00
stroller
deb34a0f8b remove useless doc for 5.0 (#9546)
Fix javadoc issues

Motivation:

Remove useless doc for 5.0

Modifications:

Remove description for 5.0

Result:

Useless doc will be removed.
2019-10-12 20:17:18 +02:00
Nick Hill
99a4cecec6 Fix NioEventLoopTest#testChannelsRegistered flakiness (#9650)
Motivation

NioEventLoopTest#testChannelsRegistered() fails intermittently due to
use of SingleThreadEventLoop#channelsRegistered() which is not
threadsafe and unreliable when called from outside the event loop.

Modifications

Add static registeredChannels method to NioEventLoopTest and
AbstractSingleThreadEventLoopTest to call from the tests via event loop
instead of directly.

Result

Hopefully fewer test failures
2019-10-09 16:57:54 +04:00
Nick Hill
170e4deee6 Fix event loop shutdown timing fragility (#9616)
Motivation

The current event loop shutdown logic is quite fragile and in the
epoll/NIO cases relies on the default 1 second wait/select timeout that
applies when there are no scheduled tasks. Without this default timeout
the shutdown would hang indefinitely.

The timeout only takes effect in this case because queued scheduled
tasks are first cancelled in
SingleThreadEventExecutor#confirmShutdown(), but I _think_ even this
isn't robust, since the main task queue is subsequently serviced which
could result in some new scheduled task being queued with much later
deadline.

It also means shutdowns are unnecessarily delayed by up to 1 second.

Modifications

- Add/extend unit tests to expose the issue
- Adjust SingleThreadEventExecutor shutdown and confirmShutdown methods
to explicitly add no-op tasks to the taskQueue so that the subsequent
event loop iteration doesn't enter blocking wait (as looks like was
originally intended)

Results

Faster and more robust shutdown of event loops, allows removal of the
default wait timeout
2019-10-07 11:06:01 +04:00
Codrut Stancu
4980a6b304 Register sun.nio.ch.SelectorImpl fields for unsafe access. (#9631)
Motivation:

On JDK > 9 Netty uses Unsafe to write two internal JDK fields: sun.nio.ch.SelectorImp.selectedKeys and sun.nio.ch.SelectorImpl.publicSelectedKeys. This is done in transport/src/main/java/io/netty/channel/nio/NioEventLoop.java:225, in openSelector() method. The GraalVM analysis cannot do the Unsafe registration automatically because the object field offset computation is hidden behind two layers of calls.

Modifications:

This PR updates the Netty GraalVM configuration by registering those fields for unsafe access.
 
Result:

Improved support for Netty on GraalVM with JDK > 9.
2019-10-07 10:10:46 +04:00
bruce
b39ffed042 Fix incorrect calculation of next buffer size in AdaptiveRecvByteBufAllocator (#9555)
Motivation:

Due a bug we did not always correctly calculate the next buffer size in AdaptiveRecvByteBufAllocator.

Modification:

Fix calculation and add unit test

Result:

Correct calculation is always used.
2019-09-27 09:59:25 +02:00
root
92941cdcac [maven-release-plugin] prepare for next development iteration 2019-09-25 06:15:31 +00:00
root
bd907c3b3a [maven-release-plugin] prepare release netty-4.1.42.Final 2019-09-25 06:14:31 +00:00
Norman Maurer
3ad037470e
Correctly reset cached local and remote address when disconnect() is called (#9545)
Motivation:

We should correctly reset the cached local and remote address when a Channel.disconnect() is called and the channel has a notion of disconnect vs close (for example DatagramChannel implementations).

Modifications:

- Correctly reset cached kicak abd remote address
- Update testcase to cover it and so ensure all transports work in a consistent way

Result:

Correctly handle disconnect()
2019-09-19 08:51:10 +02:00
stroller
d01282e5f9 Remove duplicated calculation (#9565)
Motivation:

calculateMaxBytesPerGatheringWrite() contains duplicated calculation:  getSendBufferSize() << 1

Modifications:

Remove the duplicated calculation

Result:

The method will be clear and better
2019-09-15 08:05:21 +02:00
root
01d805bb76 [maven-release-plugin] prepare for next development iteration 2019-09-12 16:09:55 +00:00
root
7cf69022d4 [maven-release-plugin] prepare release netty-4.1.41.Final 2019-09-12 16:09:00 +00:00
root
aef47bec7f [maven-release-plugin] prepare for next development iteration 2019-09-12 05:38:11 +00:00
root
267e5da481 [maven-release-plugin] prepare release netty-4.1.40.Final 2019-09-12 05:37:30 +00:00
Nizar Mankulangara
572b6a8f19 SimpleChannelPool POOL_KEY attribute name is easy to get conflict from user code (#9542) (#9548)
Motivation:

    It is noticed that SimpleChannelPool's POOL_KEY attribute name channelPool is easy to get conflict with user code and throws an exception 'channelPool' is already in use. Being a generic framework - it would be great if we can name the attribute something unique - may be use UUID for the name since the name is not required later.

    Modifications:

    This change make sure that the POOL_KEY used inside SimpleChannelPool is unique by appending the object hashcode in the name.

    Result:

    No unwanted channel attribute name conflict with user code.
2019-09-09 09:47:23 +02:00
Nitsan Wakart
d446765b84 Replace synchronized blocks + assert with synchronized method (#9538)
Motivation:

Following up on discussion with @normanmaurer with suggestion to improve code clarity.

Modification:

Method is synchronized, no need for assert or verbose sync blocks around calls.

Result:

Reduce verbosity and more idiomatic use of keyword. Also rename the method to better describe what it's for.
2019-09-05 09:12:16 +02:00
Norman Maurer
affbdf7125
Correctly protect DefaultChannelPipeline nodes when concurrent removals happen due handlerAdded(...) throwing (#9530)
Motivation:

We need to update the doubly-linked list nodes while holding a lock via synchronized in all cases as otherwise we may end-up with a corrupted pipeline. We missed this when calling remove0(...) due handlerAdded(...) throwing an exception.

Modifications:

- Correctly hold lock while update node
- Add assert
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/9528
2019-09-03 08:35:11 +02:00
Xiaoqin Fu
21b7e29ea7 Remove extra checks to fix #9456 (#9523)
Motivation:

There are some extra log level checks (logger.isWarnEnabled()).

Modification:

Remove log level checks (logger.isWarnEnabled()) from io.netty.channel.epoll.AbstractEpollStreamChannel, io.netty.channel.DefaultFileRegion, io.netty.channel.nio.AbstractNioChannel, io.netty.util.HashedWheelTimer, io.netty.handler.stream.ChunkedWriteHandler and io.netty.channel.udt.nio.NioUdtMessageConnectorChannel

Result:

Fixes #9456
2019-08-30 10:37:30 +02:00
Nick Hill
a22d4ba859 Simplify EventLoop abstractions for timed scheduled tasks (#9470)
Motivation

The epoll transport was updated in #7834 to decouple setting of the
timerFd from the event loop, so that scheduling delayed tasks does not
require waking up epoll_wait. To achieve this, new overridable hooks
were added in the AbstractScheduledEventExecutor and
SingleThreadEventExecutor superclasses.

However, the minimumDelayScheduledTaskRemoved hook has no current
purpose and I can't envisage a _practical_ need for it. Removing
it would reduce complexity and avoid supporting this specific
API indefinitely. We can add something similar later if needed
but the opposite is not true.

There also isn't a _nice_ way to use the abstractions for
wakeup-avoidance optimizations in other EventLoops that don't have a
decoupled timer.

This PR replaces executeScheduledRunnable and
wakesUpForScheduledRunnable
with two new methods before/afterFutureTaskScheduled that have slightly
different semantics:
 - They only apply to additions; given the current internals there's no
practical use for removals
 - They allow per-submission wakeup decisions via a boolean return val,
which makes them easier to exploit from other existing EL impls (e.g.
NIO/KQueue)
 - They are subjectively "cleaner", taking just the deadline parameter
and not exposing Runnables
 - For current EL/queue impls, only the "after" hook is really needed,
but specialized blocking queue impls can conditionally wake on task
submission (I have one lined up)

Also included are further optimization/simplification/fixes to the
timerFd manipulation logic.

Modifications

- Remove AbstractScheduledEventExecutor#minimumDelayScheduledTaskRemoved()
and supporting methods
- Uplift NonWakeupRunnable and corresponding default wakesUpForTask()
impl from SingleThreadEventLoop to SingleThreadEventExecutor
- Change executeScheduledRunnable() to be package-private, and have a
final impl in SingleThreadEventExecutor which triggers new overridable
hooks before/afterFutureTaskScheduled()
- Remove unnecessary use of bookend tasks while draining the task queue
- Use new hooks to add simpler wake-up avoidance optimization to
NioEventLoop (primarily to demonstrate utility/simplicity)
- Reinstate removed EpollTest class

In EpollEventLoop:
 - Refactor to use only the new afterFutureTaskScheduled() hook for
updating timerFd
 - Fix setTimerFd race condition using a monitor
 - Set nextDeadlineNanos to a negative value while the EL is awake and
use this to block timer changes from outside the EL. Restore the
known-set value prior to sleeping, updating timerFd first if necessary
 - Don't read from timerFd when processing expiry event

Result

- Cleaner API for integrating with different EL/queue timing impls
- Fixed race condition to avoid missing scheduled wakeups
- Eliminate unnecessary timerFd updates while EL is awake, and
unnecessary expired timerFd reads
- Avoid unnecessary scheduled-task wakeups when using NIO transport

I did not yet further explore the suggestion of using
TFD_TIMER_ABSTIME for the timerFd.
2019-08-21 12:34:22 +02:00
lwlee2608
cb739b2619 Update javadoc for NioEventLoop.setRatio() #9481 (#9484)
Motivation:

Improve java apidoc for NioEventLoop.setRatio

Modification:

javadoc

Result:

Fixes #9481
2019-08-21 09:13:42 +02:00
Norman Maurer
97361fa2c8
Replace synchronized usage with ConcurrentHashMap in *Bootstrap classes (#9458)
Motivation:

In AbstractBoostrap, options and attrs are LinkedHashMap that are synchronized on for every read, copy/clone, write operation.
When a lot of connections are triggered concurrently on the same bootstrap instance, the synchronized blocks lead to contention, Netty IO threads get blocked, and performance may be severely degraded.

Modifications:

Use ConcurrentHashMap

Result:

Less contention. Fixes https://github.com/netty/netty/issues/9426
2019-08-16 15:18:17 +02:00
Scott Mitchell
1fa7a5e697 EPOLL - decouple schedule tasks from epoll_wait life cycle (#7834)
Motivation:
EPOLL supports decoupling the timed wakeup mechanism from the selector call. The EPOLL transport takes advantage of this in order to offer more fine grained timer resolution. However we are current calling timerfd_settime on each call to epoll_wait and this is expensive. We don't have to re-arm the timer on every call to epoll_wait and instead only have to arm the timer when a task is scheduled with an earlier expiration than any other existing scheduled task.

Modifications:
- Before scheduled tasks are added to the task queue, we determine if the new
  duration is the soonest to expire, and if so update with timerfd_settime. We
also drain all the tasks at the end of the event loop to make sure we service
any expired tasks and get an accurate next time delay.
- EpollEventLoop maintains a volatile variable which represents the next deadline to expire. This variable is modified inside the event loop thread (before calling epoll_wait) and out side the event loop thread (immediately to ensure proper wakeup time).
- Execute the task queue before the schedule task priority queue. This means we
  may delay the processing of scheduled tasks but it ensures we transfer all
pending tasks from the task queue to the scheduled priority queue to run the
soonest to expire scheduled task first.
- Deprecate IORatio on EpollEventLoop, and drain the executor and scheduled queue on each event loop wakeup. Coupling the amount of time we are allowed to drain the executor queue to a proportion of time we process inbound IO may lead to unbounded queue sizes and unpredictable latency.

Result:
Fixes https://github.com/netty/netty/issues/7829
- In most cases this results in less calls to timerfd_settime
- Less event loop wakeups just to check for scheduled tasks executed outside the event loop
- More predictable executor queue and scheduled task queue draining
- More accurate and responsive scheduled task execution
2019-08-14 10:11:04 +02:00
root
d45a4ce01b [maven-release-plugin] prepare for next development iteration 2019-08-13 17:16:42 +00:00
root
88c2a4cab5 [maven-release-plugin] prepare release netty-4.1.39.Final 2019-08-13 17:15:20 +00:00
Dmitriy Dumanskiy
005fd55b88 #7285 Improved "Discarded inbound message" warning for embedded channel (#9414)
Motivation:

Look like `EmbeddedChannelPipeline` should also override `onUnhandledInboundMessage(ChannelHandlerContext ctx, Object msg)` in order to do not print "Discarded message pipeline" because in case of `EmbeddedChannelPipeline` discarding actually not happens.

This fixes next warning in the latest netty version with websocket and `WebSocketServerCompressionHandler`:

```
13:36:36.231 DEBUG- Decoding WebSocket Frame opCode=2
13:36:36.231 DEBUG- Decoding WebSocket Frame length=5
13:36:36.231 DEBUG- Discarded message pipeline : [JdkZlibDecoder#0, DefaultChannelPipeline$TailContext#0]. Channel : [id: 0xembedded, L:embedded - R:embedded].
```

Modification:

Override correct method

Result:
Follow up fix after https://github.com/netty/netty/pull/9286
2019-08-03 12:20:41 +02:00
root
718b7626e6 [maven-release-plugin] prepare for next development iteration 2019-07-24 09:05:57 +00:00
root
465c900c04 [maven-release-plugin] prepare release netty-4.1.38.Final 2019-07-24 09:05:23 +00:00
Dmitriy Dumanskiy
5ded050f7b #7285 Improved "Discarded inbound message" warning (#9286)
Motivation:

On servers with many pipelines or dynamic pipelines, it is easy for end user to make mistake during pipeline configuration. Current message:

`Discarded inbound message PooledUnsafeDirectByteBuf(ridx: 0, widx: 2, cap: 2) that reached at the tail of the pipeline. Please check your pipeline configuration.`

Is not always meaningful and doesn't allow to find the wrong pipeline quickly.

Modification:

Added additional log placeholder that identifies pipeline handlers and channel info. This will allow for the end users quickly find the problem pipeline.

Result:

Meaningful warning when the message reaches the end of the pipeline. Fixes #7285
2019-07-01 20:38:58 +02:00
Norman Maurer
47eb9c3bf4
Ensure sc.close() is executed before FixedChannelPoolTest.testCloseAsync() returns (#9298)
Motivation:

We observed some test-failues sometimes in the CI which happened if sc.close() was not completed before the next test did run. If this happened we would fail the bind(...) as the LocalAddress was still in use.

Modifications:

Await the close before return

Result:

Fixes race in testsuite which resulted in FixedChannelPoolTest.testAcquireNewConnection to fail if FixedChannelPoolTest.testCloseAsync() did run before it.
2019-06-29 09:21:11 +02:00
root
5b58b8e6b5 [maven-release-plugin] prepare for next development iteration 2019-06-28 05:57:21 +00:00
root
35e0843376 [maven-release-plugin] prepare release netty-4.1.37.Final 2019-06-28 05:56:28 +00:00
jimin
9621a5b981 remove unused imports (#9287)
Motivation:

Some imports are not used

Modification:

remove unused imports

Result:

Code cleanup
2019-06-26 21:08:31 +02:00
Graeme Rocher
18b7bdff12 Add null check to isSkippable. Fixes #9278 (#9280)
Motivation:

Currently GraalVM substrate returns null for reflective calls if the reflection access is not declared up front.

A change introduced in Netty 4.1.35 results in needing to register every Netty handler for reflection. This complicates matters as it is difficult to know all the possible handlers that need to be registered.

Modification:

This change adds a simple
null check such that Netty does not break on GraalVM substrate without the reflection information registration.

Result:

Fixes #9278
2019-06-25 14:45:30 +02:00
Norman Maurer
517a93d87d Make EventLoopTaskQueueFactory a top-level interface
Motivation:

c9aaa93d83 added the ability to specify an EventLoopTaskQueueFactory but did place it under MultithreadEventLoopGroup while not really belongs there.

Modifications:

Make EventLoopTaskQueueFactory a top-level interface

Result:

More logical code layout.
2019-06-22 07:38:03 +02:00
Norman Maurer
c9aaa93d83
Allow to specify a EventLoopTaskQueueFactory for various EventLoopGroup implementations (#9247)
Motivation:

Sometimes it is desirable to be able to use a different Queue implementation for the EventLoop of a Channel. This is currently not possible without resort to reflection.

Modifications:

- Add a new constructor to Nio|Epoll|KQueueEventLoopGroup which allows to specify a factory which is used to create the task queue. This was the user can override the default implementation.
- Add test

Result:

Be able to change Queue that is used for the EventLoop.
2019-06-21 09:05:19 +02:00
Divij Vaidya
fa1dedcc0f Make sync close for FixedChannelPool truly synchronous (#9226)
Motivation:

In the current implementation, the synchronous close() method for FixedChannelPool returns
after scheduling the channels to close via a single threaded executor asynchronously. Closing a channel
requires event loop group, however, there might be a scenario when the application has closed
the event loop group after the sync close() completes. In this scenario an exception is thrown
(event loop rejected the execution) when the single threaded executor tries to close the channel.

Modifications:

Complete the close function only after all the channels have been close and introduce
closeAsync() method for cases when the current/existing behaviour is desired.

Result:

Close function would completely when the channels have been closed
2019-06-14 12:01:14 +02:00
Norman Maurer
dc2649e95d
Allow to set parent Channel when constructing EmbeddedChannel (#9230)
Motivation:

Sometimes it is beneficial to be able to set a parent Channel in EmbeddedChannel if the handler that should be tested depend on the parent.

Modifications:

- Add another constructor which allows to specify a parent
- Add unit tests

Result:

Fixes https://github.com/netty/netty/issues/9228.
2019-06-08 09:11:31 -07:00
Carl Mastrangelo
67ad79d080 Handle missing methods on ChannelHandlerMask (#9221)
Motivation:

When Netty is run through ProGuard, seemingly unused methods are removed.  This breaks reflection, making the Handler skipping throw a reflective error.

Modification:

If a method is seemingly absent, just disable the optimization.

Result:

Dealing with ProGuard sucks infinitesimally less.
2019-06-07 13:39:47 -07:00
Carl Mastrangelo
dea4e33c52 Don't double record stacktrace in Annotated*Exception (#9117)
Motivation:
When initializing the AnnotatedSocketException in AbstractChannel, both
the cause and the stack trace are set, leaving a trailing "Caused By"
that is compressed when printing the trace.

Modification:
Don't include the stack trace in the exception, but leave it in the cause.

Result:
Clearer stack trace
2019-05-22 12:06:30 +02:00
Norman Maurer
f17bfd0f64
Only use static Exception instances when we can ensure addSuppressed … (#9152)
Motivation:

OOME is occurred by increasing suppressedExceptions because other libraries call Throwable#addSuppressed. As we have no control over what other libraries do we need to ensure this can not lead to OOME.

Modifications:

Only use static instances of the Exceptions if we can either dissable addSuppressed or we run on java6.

Result:

Not possible to OOME because of addSuppressed. Fixes https://github.com/netty/netty/issues/9151.
2019-05-17 22:23:02 +02:00
秦世成
cf2f1f54b6 Replace all logic that checks Null with the ObjectUtil utility class (#9145)
Motivation:

Clean the code , replace all logic that checks Null with the ObjectUtil utility class in bootstrap package

Modification:
Replace all logic that checks null with the ObjectUtil utility class

Result:

Less verbose code.
2019-05-13 19:53:45 +02:00
root
ba06eafa1c [maven-release-plugin] prepare for next development iteration 2019-04-30 16:42:29 +00:00
root
49a451101c [maven-release-plugin] prepare release netty-4.1.36.Final 2019-04-30 16:41:28 +00:00
Divij Vaidya
b9c4e17291 Invoke channelAcquired callback on first time channel acquire (#9093)
Motivation:

SimpleChannelPool provides ability to provide custom callbacks/handlers
on major events such as "channel acquired", "channel created" and
"channel released". In the current implementation, when a request to
acquire a channel is made for the first time, the internal channel pool
creates the channel lazily. This triggers the "channel created" callback
but does not invoke the "channel acquired" callback. This is contrary to
caller expectations who assumes that "channel acquired" will be invoked
at the end of every successful acquire call. It also leads to an
inconsistent API experience where the acquired callback is sometimes
invoked and sometimes it isn't depending on wheather the internal
mechanism is creating a new channel or re-using an existing one.

Modifications:

Invoke acquired callback consistenly even when creating a new channel
and modify the tests to support this behaviour

Result:

Consistent experience for the caller of acquire API. Every time they
call the API, the acquired callback will be invoked.
2019-04-29 20:45:49 +02:00
Paulo Lopes
f1495e1945 Add SVM metadata and minimal substitutions to build graalvm native image applications. (#8963)
Motivation:

GraalVM native images are a new way to deliver java applications. Netty is one of the most popular libraries however there are a few limitations that make it impossible to use with native images out of the box. Adding a few metadata (in specific modules will allow the compilation to success and produce working binaries)

Modification:

Added properties files in `META-INF` and substitutions classes (under `internal.svm`) will solve the compilation issues. The substitutions classes are not visible and do not have a public constructor so they are not visible to end users.

Result:

Fixes #8959 

This fix is very conservative as it applies the minimum config required to build:

* pure netty servers
* vert.x applications
* grpc applications

The build is having trouble due to checkstyle which does not seem to be able to find the copyright notice on property files.
2019-04-29 08:39:42 +02:00