Commit Graph

65 Commits

Author SHA1 Message Date
stroller
aa2a9931e8 Add one new constructor with threadFactory only (#9773)
Motivation:
In most cases, we want to use MultithreadEventLoopGroup such as NioEventLoopGroup without setting thread numbers but thread name only. So we need to use followed code:
NioEventLoopGroup boss = new NioEventLoopGroup(0, new DefaultThreadFactory("boss"));
It looks a bit confuse or strange for the number 0 due to we only want to set thread name. So it will be better to add new constructor for this case.

Modifications:
add new constructor into all event loop groups, for example: public NioEventLoopGroup(ThreadFactory threadFactory)

Result:
User can only set thread factory without setting the thread number to 0:
NioEventLoopGroup boss = new NioEventLoopGroup(new DefaultThreadFactory("boss"));
2019-11-18 09:42:44 +01:00
Joe Ellis
aebe2064d5 Allow domain sockets to configure SO_SNDBUF and SO_RCVBUF (#9584)
Motivation:

Running tests with a `KQueueDomainSocketChannel` showed worse performance than an `NioSocketChannel`. It turns out that the default send buffer size for Nio sockets is 64k while for KQueue sockets it's 8k. I verified that manually setting the socket's send buffer size improved perf to expected levels.

Modification:

Plumb the `SO_SNDBUF` and `SO_RCVBUF` options into the `*DomainSocketChannelConfig`.

Result:

Can now configure send and receive buffer sizes for domain sockets.
2019-09-20 22:28:53 +02: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
Norman Maurer
394a1b3485
Add support for recvmmsg when using epoll transport (#9509)
Motivation:

When using datagram sockets which need to handle a lot of packets it makes sense to use recvmmsg to be able to read multiple datagram packets with one syscall.

Modifications:

- Add support for recvmmsg on linux
- Add new EpollChannelOption.MAX_DATAGRAM_PACKET_SIZE
- Add tests

Result:

Fixes https://github.com/netty/netty/issues/8446.
2019-09-03 08:40:17 +02:00
jingene
c0f9364870 Change the netty.io homepage scheme(http -> https) (#9344)
Motivation:

Netty homepage(netty.io) serves both "http" and "https".
It's recommended to use https than http.
Modification:

I changed from "http://netty.io" to "https://netty.io"
Result:

No effects.
2019-07-09 21:09:42 +02:00
jimin
856f1185e1 All override methods must be added @override (#9285)
Motivation:

Some methods that either override others or are implemented as part of implementation an interface did miss the `@Override` annotation

Modifications:

Add missing `@Override`s

Result:

Code cleanup
2019-06-27 13:51:26 +02: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
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
Julien Viet
e348bd9217 KQueueEventLoop | EpollEventLoop may incorrectly update registration when FD is reused.
Motivation:

The current KQueueEventLoop implementation does not process concurrent domain socket channel registration/unregistration in the order they actual
happen since unregistration are delated by an event loop task scheduling. When a domain socket is closed, it's file descriptor might be reused
quickly and therefore trigger a new channel registration using the same descriptor.

Consequently the KQueueEventLoop#add(AbstractKQueueChannel) method will overwrite the current inactive channels having the same descriptor
and the delayed KQueueEventLoop#remove(AbstractKQueueChannel) will remove the active channel that replaced the inactive one.

As active channels are registered, events for this file descriptor won't be processed anymore and the channels will never be closed.

The same problem can also happen in EpollEventLoop. Beside this we also may never remove the AbstractEpollChannel from the internal map
when it is unregistered which will prevent it from be GC'ed

Modifications:

- Change logic of native KQueue and Epoll implementations to ensure we correctly handle the case of FD reuse
- Only try to update kevent / epoll if the Channel is still open (as otherwise it will be handled by kqueue / epoll itself)
- Correctly remove AbstractEpollChannel from internal map in all cases
- Make implementation of closeAll() consistent for Epoll and KQueueEventLoop

Result:

KQueue and Epoll native transports correctly handle FD reuse

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
2019-05-22 09:23:09 +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
Norman Maurer
71c184076c Revert "KQueueEventLoop won't unregister active channels reusing a file descriptor (#9114)"
This reverts commit 909a3d942e.
2019-05-07 16:44:41 +02:00
Julien Viet
909a3d942e KQueueEventLoop won't unregister active channels reusing a file descriptor (#9114)
Motivation:

The current KQueueEventLoop implementation does not process concurrent domain socket channel registration/unregistration in the order they actual
happen since unregistration are delated by an event loop task scheduling. When a domain socket is closed, it's file descriptor might be reused
quickly and therefore trigger a new channel registration using the same descriptor.

Consequently the KQueueEventLoop#add(AbstractKQueueChannel) method will overwrite the current inactive channels having the same descriptor
and the delayed KQueueEventLoop#remove(AbstractKQueueChannel) will remove the active channel that replaced the inactive one.

As active channels are registered, events for this file descriptor won't be processed anymore and the channels will never be closed.

Modifications:

Change the logic of KQueueEventLoop#remove(AbstractKQueueChannel) channels so it will check channels equality prior removal.

Result:

KQueueEventLoop won't remove anymore active channels reusing a file descriptor.
2019-05-07 10:19:42 +02:00
Nick Hill
9ed41db1d7 Have (Epoll|KQueue)RecvByteAllocatorHandle extend DelegatingHandle (#9060)
Motivation

These implementations delegate most of their methods to an existing Handle and previously extended RecvByteBufAllocator.DelegatingHandle. This was reverted in #6322 with the introduction of ExtendedHandle but it's not clear to me why it needed to be - the code looks a lot cleaner.

Modifications

Have (Epoll|KQueue)RecvByteAllocatorHandle extend DelegatingHandle again, while still implementing ExtendedHandle.

Result

Less code.
2019-04-16 09:14:09 +02:00
Vladimir Kostyukov
0a0da67f43 Introduce SingleThreadEventLoop.registeredChannels (#8428)
Motivation:

Systems depending on Netty may benefit (telemetry, alternative even loop scheduling algorithms) from knowing the number of channels assigned to each EventLoop.

Modification:

Expose the number of channels registered in the EventLoop via SingleThreadEventLoop.registeredChannels.

Result:

Fixes #8276.
2019-03-28 11:33:12 +00:00
Lunfu Zhong
e7b3195570 Support ALLOW_HALF_CLOSURE channel option on Unix domain socket. (#8932)
Motivation:

Since DomainSocketChannel is a DuplexChannel,  which be able to shutdown input or output individually on demands, but ALLOW_HALF_CLOSURE channel option has not been supported yet.

I thought this could be a missing feature of Unix domain socket, so here the PR for it.

Modifications:

1. Added allHalfClosure property both in  EpollDomainSocketChannelConfig and KQueueDomainSocketChannelConfig,
2. Enabled isAllowHalfClosure method of native channel to support domain channel config,
3. Created EpollDomainSocketShutdownOutputByPeerTest and KQueueDomainSocketShutdownOutputByPeerTest to verify the change.

Result:

ALLOW_HALF_CLOSURE channel option can be set with DomainSocketChannel, and no more warning of Unknown channel option 'ALLOW_HALF_CLOSURE'.
2019-03-19 11:24:07 +01:00
Norman Maurer
81e43d5088
DefaultFileRegion.transferTo with invalid count may cause busy-spin (#8885)
Motivation:

`DefaultFileRegion.transferTo` will return 0 all the time when we request more data then the actual file size. This may result in a busy spin while processing the fileregion during writes.

Modifications:

- If we wrote 0 bytes check if the underlying file size is smaller then the requested count and if so throw an IOException
- Add DefaultFileRegionTest
- Add a test to the testsuite

Result:

Fixes https://github.com/netty/netty/issues/8868.
2019-02-26 11:08:09 +01:00
Norman Maurer
d02b51965f
Don't deregister Channel as part of closing it when using native kqueue transport (#8881)
Motivation:

In https://github.com/netty/netty/pull/8665 we changed how we handle the registration of Channels to KQueue but missed to removed some code which would deregister the Channel before it actual closed the underlying socket. This could lead to have events triggered still while not have a mapping to the Channel anymore.

Modifications:

Remove deregister call during socket closure.

Result:

Fixes https://github.com/netty/netty/issues/8849.
2019-02-25 08:55:55 +01:00
Norman Maurer
7bba4f49cf
Reduce GC produced by native DatagramChannel implementations when in connected mode. (#8806)
Motivation:

In the native code EpollDatagramChannel / KQueueDatagramChannel creates a DatagramSocketAddress object for each received UDP datagram even when in connected mode as it uses the recvfrom(...) / recvmsg(...)  method. Creating these is quite heavy in terms of allocations as internally, char[], String, Inet4Address, InetAddressHolder, InetSocketAddressHolder, InetAddress[], byte[] objects are getting generated when constructing the object. When in connected mode we can just use regular read(...) calls which do not need to allocate all of these.

Modifications:

- When in connected mode use read(...) and NOT recvfrom(..) / readmsg(...) to reduce allocations when possible.
- Adjust tests to ensure read works as expected when in connected mode.

Result:

Less allocations and GC when using native datagram channels in connected mode. Fixes https://github.com/netty/netty/issues/8770.
2019-02-01 10:29:36 +01:00
田欧
a33200ca38 use checkPositive/checkPositiveOrZero (#8803)
Motivation:

We have a utility method to check for > 0 and >0 arguments. We should use it.

Modification:

use checkPositive/checkPositiveOrZero instead of if statement.

Result:

Re-use utility method.
2019-01-31 09:07:14 +01:00
Norman Maurer
cd3254df88
Update to new checkstyle plugin (#8777) (#8780)
Motivation:

We need to update to a new checkstyle plugin to allow the usage of lambdas.

Modifications:

- Update to new plugin version.
- Fix checkstyle problems.

Result:

Be able to use checkstyle plugin which supports new Java syntax.
2019-01-25 11:58:42 +01:00
kashike
6fdd7fcddb Fix minor spelling issues in javadocs (#8701)
Motivation:

Javadocs contained some spelling errors, we should fix these.

Modification:

Fix spelling

Result:

Javadoc cleanup.
2019-01-14 07:24:34 +01:00
Norman Maurer
d77bdeaa7d
Fix ClassCastException and native crash when using kqueue transport. (#8665)
Motivation:

How we did the mapping from native code to AbstractKQueueChannel was not safe and could lead to heap corruption. This then sometimes produced ClassCastExceptions or could also lead to crashes. This happened sometimes when running the testsuite.

Modifications:

Use a Map for the mapping (just as we do in the native epoll transport).

Result:

No more heap corruption / crashes.
2018-12-19 12:13:56 +01:00
Matteo Merli
3a96e7373b Added option to do busy-wait on epoll (#8267)
Motivation:

Add an option (through a SelectStrategy return code) to have the Netty event loop thread to do busy-wait on the epoll.

The reason for this change is to avoid the context switch cost that comes when the event loop thread is blocked on the epoll_wait() call.

On average, the context switch has a penalty of ~13usec.

This benefits both:

The latency when reading from a socket
Scheduling tasks to be executed on the event loop thread.
The tradeoff, when enabling this feature, is that the event loop thread will be using 100% cpu, even when inactive.

Modification:

Added SelectStrategy option to return BUSY_WAIT
Epoll loop will do a epoll_wait() with no timeout
Use pause instruction to hint to processor that we're in a busy loop
Result:

When enabled, minimizes impact of context switch in the critical path
2018-09-28 22:52:00 +02:00
Roger
6138541033 Avoid repeating the same field and hiding it (#8335)
Motivation

The EpollChannelConfig (same for KQueues) and its subclasses repeatetly declare their own channel field which leads to a 3x repetition for each config instance. Given the fields are protected or package-private it's exposing the code code to "field hiding" bugs.

Modifications

Use the the existing protected channel field from the DefaultChannelConfig class and simply cast it when needed.

Result

Fixes #8331
2018-09-28 17:37:14 +02:00
Norman Maurer
54f565ac67
Allow to use native transports when sun.misc.Unsafe is not present on… (#8231)
* Allow to use native transports when sun.misc.Unsafe is not present on the system

Motivation:

We should be able to use the native transports (epoll / kqueue) even when sun.misc.Unsafe is not present on the system. This is especially important as Java11 will be released soon and does not allow access to it by default.

Modifications:

- Correctly disable usage of sun.misc.Unsafe when -PnoUnsafe is used while running the build
- Correctly increment metric when UnpooledDirectByteBuf is allocated. This was uncovered once -PnoUnsafe usage was fixed.
- Implement fallbacks in all our native transport code for when sun.misc.Unsafe is not present.

Result:

Fixes https://github.com/netty/netty/issues/8229.
2018-08-29 19:36:33 +02:00
Ziyan Mo
785473788f (Nio|Epoll)EventLoop.pendingTasks does not need to dispatch to the EventLoop (#8197)
Motivation:

EventLoop.pendingTasks should be (reasonably) cheap to invoke so it can be used within observability. 

Modifications:

Remove code that dispatch access to the internal taskqueue to the EventLoop when invoked as this is not needed anymore with the current MPSC queues we are using. 

See https://github.com/netty/netty/issues/8196#issuecomment-413653286.

Result:

Fixes https://github.com/netty/netty/issues/8196
2018-08-18 07:28:31 +02:00
Scott Mitchell
12f6500a4f Epoll and Kqueue shouldn't read by default (#8024)
Motivation:
Epoll and Kqueue channels have internal state which forces
a single read operation after channel construction. This
violates the Channel#read() interface which indicates that
data shouldn't be delivered until this method is called.
The behavior is also inconsistent with the NIO transport.

Modifications:
- Epoll and Kqueue shouldn't unconditionally read upon
initialization, and instead should rely upon Channel#read()
or auto_read.

Result:
Epoll and Kqueue are more consistent with NIO.
2018-06-15 10:28:50 +02:00
Norman Maurer
d133bf06a4
Allow to schedule tasks up to Long.MAX_VALUE (#7972)
Motivation:

We should allow to schedule tasks with a delay up to Long.MAX_VALUE as we did pre 4.1.25.Final.

Modifications:

Just ensure we not overflow and put the correct max limits in place when schedule a timer. At worse we will get a wakeup to early and then schedule a new timeout.

Result:

Fixes https://github.com/netty/netty/issues/7970.
2018-05-30 11:11:42 +02:00
Norman Maurer
030318e53c
Read until all data is consumed when EOF is detected even if readPend… (#7961)
* Read until all data is consumed when EOF is detected even if readPending is false and auto-read is disabled.

Motivation:

We should better always notify the user of EOF even if the user did not request any data as otherwise we may never be notified when the remote peer closes the connection. This should be ok as the amount of extra data we may read and so fire through the pipeline is limited by SO_RECVBUF.

Modifications:

- Always drain the socket when EOF is detected.
- Add testcase

Result:

No risk for the user to be not notified of EOF.
2018-05-24 20:29:29 +02:00
Norman Maurer
358249e5c9
Allow to disable native transport and native ssl support via system property. (#7903)
Motivation:

Sometimes it's useful to disable native transports / native ssl to debug a problem. We should allow to do so with a system property so people not need to adjust code for this.

Modifications:

Add system properties which allow to disable native transport and native ssl.

Result:

Easier to disable native code usage without code changes.
2018-05-04 14:44:44 +02:00
Norman Maurer
b47fb81799
EventLoop.schedule with big delay fails (#7402)
Motivation:

Using a very huge delay when calling schedule(...) may cause an Selector error when calling select(...) later on. We should gaurd against such a big value.

Modifications:

- Add guard against a very huge value.
- Added tests.

Result:

Fixes [#7365]
2018-04-24 11:15:20 +02:00
Scott Mitchell
ed0668384b NIO read spin event loop spin when half closed (#7801)
Motivation:
AbstractNioByteChannel will detect that the remote end of the socket has
been closed and propagate a user event through the pipeline. However if
the user has auto read on, or calls read again, we may propagate the
same user events again. If the underlying transport continuously
notifies us that there is read activity this will happen in a spin loop
which consumes unnecessary CPU.

Modifications:
- AbstractNioByteChannel's unsafe read() should check if the input side
of the socket has been shutdown before processing the event. This is
consistent with EPOLL and KQUEUE transports.
- add unit test with @normanmaurer's help, and make transports consistent with respect to user events

Result:
No more read spin loop in NIO when the channel is half closed.
2018-03-28 20:02:57 +02:00
Norman Maurer
0a8e1aaf19 Flush task should not flush messages that were written since last flush attempt.
Motivation:

The flush task is currently using flush() which will have the affect of have the flush traverse the whole ChannelPipeline and also flush messages that were written since we gave up flushing. This is not really correct as we should only continue to flush messages that were flushed at the point in time when the flush task was submitted for execution if the user not explicit call flush() by him/herself.

Modification:

Call *Unsafe.flush0() via the flush task which will only continue flushing messages that were marked as flushed before.

Result:

More correct behaviour when the flush task is used.
2018-03-02 10:09:40 +09:00
Scott Mitchell
d2d3e6ef0c
KQueue write filter initial state (#7738)
Motivation:
KQueue implementations current have inconsistent behavior with Epoll implementations with respect to asynchronous sockets and connecting. In the Epoll transport we attempt to connect, if the connect call does not synchornously fail/succeed we set the EPOLLOUT which will be triggered by the kernel if the connection attempt succeeds or an error occurs. The connect API provides no way to asynchronously communicate an error so the Epoll implementation fires a EPOLLOUT event and puts the connect status in getsockopt(SO_ERROR). KQueue provides the same APIs but different behavior. If the EVFILT_WRITE is not enabled and the EVFILT_READ is enabled before connect is called, and there is an error the kernel may fire the EVFILT_READ filter and provide the Connection Refused error via read(). This is even true if we set the EVFILT_WRITE filter after calling connect because connect didn't synchornously complete. After the error has been delievered via read() a call to getsockopt(SO_ERROR) will return 0 indicating there is no error. This means we cannot rely upon the KQueue based kernel to deliver connection errors via the EVFILT_WRITE filter in the same way that the linux kernel does with the EPOLLOUT flag.
ce241bd introduced a change which depends upon the behavior of the EVFILT_WRITE being set and may prematurely stop writing to the OS as a result, becaues we assume the OS will notify us when the socket is writable. However the current work around for the above described behavior is to initialize the EVFILT_WRITE to true for connection oriented protocols. This leads to prematurely exiting from the flush() which may lead to deadlock.

Modifications:
- KQueue should check when an error is obtained from read() if the connectPromise has not yet been completed, and if not complete it with a ConnectException

Result:
No more deadlock in KQueue due to asynchronous connect workaround.
2018-02-20 11:01:49 -08:00
Scott Mitchell
ce241bd11e Epoll flush/writabilityChange deadlock
Motivation:
b215794de3 recently introduced a change in behavior where writeSpinCount provided a limit for how many write operations were attempted per flush operation. However when the write quantum was meet the selector write flag was not cleared, and the channel unsafe flush0 method has an optimization which prematurely exits if the write flag is set. This may lead to no write progress being made under the following scenario:
- flush is called, but the socket can't accept all data, we set the write flag
- the selector wakes us up because the socket is writable, we write data and use the writeSpinCount quantum
- we then schedule a flush() on the EventLoop to execute later, however it the flush0 optimization prematurely exits because the write flag is still set

In this scenario the socket is still writable so the EventLoop may never notify us that the socket is writable, and therefore we may never attempt to flush data to the OS.

Modifications:
- When the writeSpinCount quantum is exceeded we should clear the selector write flag

Result:
Fixes https://github.com/netty/netty/issues/7729
2018-02-20 11:40:58 +01:00
Scott Mitchell
b215794de3
Enforce writeSpinCount to limit resource consumption per socket (#7478)
Motivation:
The writeSpinCount currently loops over the same buffer, gathering
write, file write, or other write operation multiple times but will
continue writing until there is nothing left or the OS doesn't accept
any data for that specific write. However if the OS keeps accepting
writes there is no way to limit how much time we spend on a specific
socket. This can lead to unfair consumption of resources dedicated to a
single socket.
We currently don't limit the amount of bytes we attempt to write per
gathering write. If there are many more bytes pending relative to the
SO_SNDBUF size we will end up building iov arrays with more elements
than can be written, which results in extra iteration, conditionals,
and book keeping.

Modifications:
- writeSpinCount should limit the number of system calls we make to
write data, instead of applying to individual write operations
- IovArray should support a maximum number of bytes
- IovArray should support composite buffers of greater than size 1024
- We should auto-scale the amount of data that we attempt to write per
gathering write operation relative to SO_SNDBUF and how much data is
successfully written
- The non-unsafe path should also support a maximum number of bytes,
and respect the IOV_MAX limit

Result:
Write resource consumption can be bounded and gathering writes have
a limit relative to the amount of data which can actually be accepted
by the socket.
2017-12-07 16:00:52 -08:00
Norman Maurer
3f101caa4c Not call java methods from within JNI init code to prevent class loading deadlocks.
Motivation:

We used NetUtil.isIpV4StackPreferred() when loading JNI code which tries to load NetworkInterface in its static initializer. Unfortunally a lock on the NetworkInterface class init may be already hold somewhere else which may cause a loader deadlock.

Modifications:

Add a new Socket.initialize() method that will be called when init the library and pass everything needed to the JNI level so we not need to call back to java.

Result:

Fixes [#7458].
2017-12-06 14:34:15 +01:00
Norman Maurer
251bb1a739 Not use safeRelease(...) but release(...) to release non-readable holders to ensure we not mask errors.
Motivation:

AbstractChannel attempts to "filter" messages which are written [1]. A goal of this process is to copy from heap to direct if necessary. However implementations of this method [2][3] may translate a buffer with 0 readable bytes to EMPTY_BUFFER. This may mask a user error where an empty buffer is written but already released.

Modifications:

Replace safeRelease(...) with release(...) to ensure we propagate reference count issues.

Result:

Fixes [#7383]
2017-12-04 20:38:35 +01:00
Norman Maurer
e7f02b1dc0 Set readPending to false when EOF is detected while issue an read
Motivation:

We need to set readPending to false when we detect a EOF while issue a read as otherwise we may not unregister from the Selector / Epoll / KQueue and so keep on receving wakeups.

The important bit is that we may even get a wakeup for a read event but will still will only be able to read 0 bytes from the socket, so we need to be very careful when we clear the readPending. This can happen because we generally using edge-triggered mode for our native transports and because of the nature of edge-triggered we may schedule an read event just to find out there is nothing left to read atm (because we completely drained the socket on the previous read).

Modifications:

Set readPending to false when EOF is detected.

Result:

Fixes [#7255].
2017-11-06 15:44:36 -08:00
Norman Maurer
bcad9dbf97 Revert "Set readPending to false when ever a read is done"
This reverts commit 413c7c2cd8 as it introduced an regression when edge-triggered mode is used which is true for our native transports by default. With 413c7c2cd8 included it was possible that we set readPending to false by mistake even if we would be interested in read more.
2017-11-06 09:21:42 -08:00
Scott Mitchell
413c7c2cd8 Set readPending to false when ever a read is done
Motivation:
readPending is currently only set to false if data is delivered to the application, however this may result in duplicate events being received from the selector in the event that the socket was closed.

Modifications:
- We should set readPending to false before each read attempt for all
transports besides NIO.
- Based upon the Javadocs it is possible that NIO may have spurious
wakeups [1]. In this case we should be more cautious and only set
readPending to false if data was actually read.

[1] https://docs.oracle.com/javase/7/docs/api/java/nio/channels/SelectionKey.html
That a selection key's ready set indicates that its channel is ready for some operation category is a hint, but not a guarantee, that an operation in such a category may be performed by a thread without causing the thread to block.

Result:
Notification from the selector (or simulated events from kqueue/epoll ET) in the event of socket closure.
Fixes https://github.com/netty/netty/issues/7255
2017-10-25 08:25:54 -07:00
Idel Pivnitskiy
50a067a8f7 Make methods 'static' where it possible
Motivation:

Even if it's a super micro-optimization (most JVM could optimize such
 cases in runtime), in theory (and according to some perf tests) it
 may help a bit. It also makes a code more clear and allows you to
 access such methods in the test scope directly, without instance of
 the class.

Modifications:

Add 'static' modifier for all methods, where it possible. Mostly in
test scope.

Result:

Cleaner code with proper 'static' modifiers.
2017-10-21 14:59:26 +02:00
Norman Maurer
9bcf31977c Fail the connectPromise with the correct exception if the connection is refused when using the native kqueue transport.
Motivation:

Due a bug we happen to sometimes fail the connectPromise with a ClosedChannelException when using the kqueue transport and the remote peer refuses the connection. We need to ensure we fail it with the correct exception.

Modifications:

Call finishConnect() before calling close() to ensure we preserve the correct exception.

Result:

KQueueSocketConnectionAttemptTest.testConnectionRefused will pass always on macOS.
2017-10-07 21:33:26 +02:00
Carl Mastrangelo
d3ca087f6b Propagate all exceptions when loading native code
Motivation:
There are 2 motivations, the first depends on the second:

Loading Netty Epoll statically stopped working in 4.1.16, due to
`Native` always loading the arch specific shared object.  In a
static binary, there is no arch specific SO.

Second, there are a ton of exceptions that can happen when loading
a native library.  When loading native code, Netty tries a bunch of
different paths but a failure in any given may not be fatal.

Additionally: turning on debug logging is not always feasible so
exceptions get silently swallowed.

Modifications:

* Change Epoll and Kqueue to try the static load second
* Modify NativeLibraryLoader to record all the locations where
  exceptions occur.
* Attempt to use `addSuppressed` from Java 7 if available.

Alternatives Considered:

An alternative would be to record log messages at each failure.  If
all load attempts fail, the log messages are printed as warning,
else as debug. The problem with this is there is no `LogRecord` to
create like in java.util.logging.  Buffering the args to
logger.log() at the end of the method loses the call site, and
changes the order of events to be confusing.

Another alternative is to teach NativeLibraryLoader about loading
the SO first, and then the static version.  This would consolidate
the code fore Epoll, Kqueue, and TCNative.   I think this is the
long term better option, but this PR is changing a lot already.
Someone else can take a crack at it later

Results:
Epoll Still Loads and easier debugging.
2017-10-04 08:45:27 +02:00
Norman Maurer
aa8bdb5d6b Fix assertion error when closing / shutdown native channel and SO_LINGER is set.
Motivation:

When SO_LINGER is used we run doClose() on the GlobalEventExecutor by default so we need to ensure we schedule all code that needs to be run on the EventLoop on the EventLoop in doClose. Beside this there are also threading issues when calling shutdownOutput(...)

Modifications:

- Schedule removal from EventLoop to the EventLoop
- Correctly handle shutdownOutput and shutdown in respect with threading-model
- Add unit tests

Result:

Fixes [#7159].
2017-09-18 14:46:37 -07:00
Norman Maurer
0fffc844d6 Only load native transport if running architecture match the compiled library architecture.
Motivation:

We should only try to load the native artifacts if the architecture we are currently running on is the same as the one the native libraries were compiled for.

Modifications:

Include architecture in native lib name and append the current arch when trying to load these. This will fail then if its not the same as the arch of the compiled arch.

Result:

Fixes [#7150].
2017-09-04 13:34:55 +02:00
Carl Mastrangelo
c891c9c13f Include more detail why Unsafe is not available
Motivation:
PD and PD0 Both try to find and use Unsafe.  If unavailable, they
try to log why and continue on.  However, it is not always east to
enable this logging.  Chaining exceptions together is much easier
to reach, and the original exception is relevant when Unsafe is
needed.

Modifications:
* Make PD log why PD0 could not be loaded with a trace level log
* Make PD0 remember why Unsafe wasn't available
* Expose unavailability cause through PD for higher level use.
* Make Epoll and KQueue include the reason when failing

Result:
Easier debugging in hard to reconfigure environments
2017-08-29 22:02:06 +02:00
Scott Mitchell
89ecb4b4a4 AutoClose behavior may infinite loop
Motivation:
If AutoClose is false and there is a IoException then AbstractChannel will not close the channel but instead just fail flushed element in the ChannelOutboundBuffer. AbstractChannel also notifies of writability changes, which may lead to an infinite loop if the peer has closed its read side of the socket because we will keep accepting more data but continuously fail because the peer isn't accepting writes.

Modifications:
- If the transport throws on a write we should acknowledge that the output side of the channel has been shutdown and cleanup. If the channel can't accept more data because it is full, and still healthy it is not expected to throw. However if the channel is not healthy it will throw and is not expected to accept any more writes. In this case we should shutdown the output for Channels that support this feature and otherwise just close.
- Connection-less protocols like UDP can remain the same because the channel may disconnected temporarily.
- Make sure AbstractUnsafe#shutdownOutput is called because the shutdown on the socket may throw an exception.

Result:
More correct handling of write failure when AutoClose is false.
2017-08-25 21:01:41 -07:00
Scott Mitchell
fe2dd973e9 Unify KQueue and Epoll wait timeout approach
Motivation:
KQueueEventLoop and EpollEventLoop implement different approaches to applying a timeout of their respective poll calls. Epoll attempts to ensure the desired timeout is satisfied at the java layer and at the JNI layer, but it should be sufficient to account for spurious wakups at the JNI layer. Epoll timeout granularity is also limited to milliseconds which may be too large for some latency sensitive applications.

Modifications:
- Make EpollEventLoop wait method look like KQueueEventLoop
- Epoll should support a finer timeout granularity via timerfd_create. We can hide most of these details behind the epollWait0 JNI call to avoid crossing additional JNI boundaries.

Result:
More consistent timeout approach between KQueue and Epoll.
2017-08-18 13:09:02 -07:00