Motivation:
We had a lot of duplication in our jni code which was mostly due macros but also related to how we support shading. By using netty-jni-util we can share all the code between netty and netty-tcnative ( and possible other jni based netty projects in the future).
Modifications:
- Use netty-jni-util and re-use its macros / functions
- Remove duplicated code
- Adjust build files
Result:
Less code duplication for JNI
Motivation:
It's important to unload all previous registered native methods when there is a failure during loading the native lib. Failing to do so may lead to an "invalid state" and so may segfault the JVM when trying to call a native method that was previous loaded.
This was observed when two versions of netty-tcnative were on the classpath which had different requirements in terms of linking.
Something like this was reported in he hs log:
```
Instructions: (pc=0x0000000116413bf0)
0x0000000116413bd0:
[error occurred during error reporting (printing registers, top of stack, instructions near pc), id 0xb]
Register to memory mapping:
RAX=0x0000000116413bf0 is an unknown value
RBX={method} {0x000000011422e708} 'aprMajorVersion' '()I' in 'io/netty/internal/tcnative/Library'
RCX=0x000000000000000a is an unknown value
RDX=0x000000000000000a is an unknown value
```
Modifications:
- Unregister previous registered native methods on failure
- Unregister previous registered native methods on on unload of the native lib
Result:
No more segfault caused by invalid state when loading of the native lib fails in between. In this case the user will receive an error now like:
Motivation:
Thread.stop() works by producing a ThreadDeath error in the target thread. EventLoops swallow all Throwables, which makes them effectively unkillable. This is effectively a memory leak, for our application. Beside this we should also just regrow all `Error` as there is almost no way to recover.
Modification:
Edit the EventLoops that swallow Throwables to instead rethrow Error.
Result:
`EventLoop` can crash if `Error` is thrown
Motivation:
HTTP is a plaintext protocol which means that someone may be able
to eavesdrop the data. To prevent this, HTTPS should be used whenever
possible. However, maintaining using https:// in all URLs may be
difficult. The nohttp tool can help here. The tool scans all the files
in a repository and reports where http:// is used.
Modifications:
- Added nohttp (via checkstyle) into the build process.
- Suppressed findings for the websites
that don't support HTTPS or that are not reachable
Result:
- Prevent using HTTP in the future.
- Encourage users to use HTTPS when they follow the links they found in
the code.
Motivation:
DuplexChannel allow for half-closure, we should have a special config interface for it as well.
Modifications:
Add DuplexChannelConfig which allows to configure half-closure.
Result:
More consistent types
Motivation:
LGTM reports multiple issues. They need to be triaged,
and real ones should be fixed.
Modifications:
- Fixed multiple issues reported by LGTM, such as redundant conditions,
resource leaks, typos, possible integer overflows.
- Suppressed false-positives.
- Added a few testcases.
Result:
Fixed several possible issues, get rid of false alarms in the LGTM report.
Motivation:
All scheduled executors should behave in accordance to their API.
The bug here is that scheduled tasks were not run more than once because we executed the runnables directly, instead of through the provided runnable future.
Modification:
We now run tasks through the provided future, so that when each run completes, the internal state of the task is reset and the ScheduledThreadPoolExecutor is informed of the completion.
This allows the executor to prepare the next run.
Result:
The UnorderedThreadPoolEventExecutor is now able to run scheduled tasks more than once.
Which is what one would expect from the API.
Motivation:
Creating exceptions is expensive so we should only do so if really needed.
Modifications:
Only create the ConnectTimeoutException if we really need it.
Result:
Less overhead
Motivation:
When we try to parse the kernel version we need to be careful what to
expect. Especially when a custom kernel is used we may get extra chars
in the version numbers. For example I had this one fail because of my
custom kernel that I built for io_uring:
5.8.7ioring-fixes+
Modifications:
- Try to be a bit more lenient when parsing
- If we cant parse the kernel version just use 0.0.0
Result:
Tests are more robust
only
Motivation:
4b7dba1 introduced a change which was not 100 % complete and so
introduce a regression when a user specified to use
InetProtocolFamily.IPv4 and trying to bind to a port (without specify
the ip).
Modifications:
- Fix regression by respect the InetProtocolFamily
- Add unit test
Result:
Fix regression when binding to port explicit
Motivation:
When we were using the netty http protocol, OOM occurred, this problem has been in 4.1.51.Final Fix [# 10424](https://github.com/netty/netty/issues/10424), even if OOM is up, the service will still receive new connection events, will occur again OOM and eventually cause the connection not to be released.
code `byteBuf = allocHandle.allocate(allocator);`
Modification:
I fail to create buffer when I try to receive new data, i determine if it is OOM then the close read event releases the connection.
```java
if (close || cause instanceof OutOfMemoryError || cause instanceof IOException) {
closeOnRead(pipeline);
}
```
Result:
Fixes # [10434](https://github.com/netty/netty/issues/10434).
Motivation:
Even if the system does not support ipv6 we should try to use it if the user explicit pass an Inet6Address. This way we ensure we fail and not try to convert this to an ipv4 address internally.
This incorrect behavior was introduced by 70731bfa7e
Modifications:
If the user explicit passed an Inet6Address we force the usage of ipv6
Result:
Fixes https://github.com/netty/netty/issues/10402
Motivation:
`transport-native-epoll` doesn't have ARM release package.
Modification:
This PR added cross compile profile for epoll. Then we can easily build aarch64 package on X86 machine.
Result:
Fixes#8279
Motivation:
netty_epoll_linuxsocket_JNI_OnLoad(...) may produce a deadlock with another thread that will load IOUtil in a static block. This seems to be a JDK bug which is not yet fixed. To workaround this we force IOUtil to be loaded from without java code before init the JNI code
Modifications:
Use Selector.open() as a workaround to load IOUtil
Result:
Fixes https://github.com/netty/netty/issues/10187
Motivation:
8dc6ad5 introduced IPV6-mapped-IPV4 address support but
copied the addresses incorrectly. It copied the first
4 bytes of the ipv6 address to the address byte array
at offset 12, instead of the other way around.
7a547aa implemented this correctly in netty_unix_socket.c
but it seems the change should've been applied to
netty_epoll_native.c as well.
The current behaviour will always set the address to
`0.0.0.0`.
Modifications:
Copy the correct bytes from the ipv6 mapped ipv4 address.
I.e. copy 4 bytes at offset 12 from the native address
to the byte array `addr` at offset 0.
Result:
When using recvmmsg with IPV6-mapped-IPV4 addresses,
the address will be correctly copied to the byte array
`addr` in the NativeDatagramPacket instance.
Motivation:
https://github.com/netty/netty/pull/9797 changed the code for recvmmsg and sendmmsg to use the syscalls directly to remvove the dependency on newer GLIBC versions. Unfortunally it made the assumption that the syscall numbers are the same for different architectures, which is not the case.
Thanks to @jayv for pointing it out
Modifications:
Add #if, #elif and #else declarations to ensure we pick the correct syscall number (or not support if if the architecture is not supported atm).
Result:
Pick the correct syscall number depending on the architecture.
Motivation:
Due a bug we did not correctly set the writerIndex of the ByteBuf when a
user specified EpollChannelOption.MAX_DATAGRAM_PAYLOAD_SIZE but we ended
up with a non scattering read.
Modifications:
- Set writerIndex to the correct value
- Add unit tests
Result:
Fixes https://github.com/netty/netty/issues/9788
Motivation
The event loop implementations had become somewhat tangled over time and
work was done recently to streamline EpollEventLoop. NioEventLoop would
benefit from the same treatment and it is more straighforward now that
we can follow the same structure as was done for epoll.
Modifications
Untangle NioEventLoop logic and mirror what's now done in EpollEventLoop
w.r.t. the volatile selector wake-up guard and scheduled task deadline
handling.
Some common refinements to EpollEventLoop have also been included - to
use constants for the "special" deadline/wakeup volatile values and to
avoid some unnecessary calls to System.nanoTime() on task-only
iterations.
Result
Hopefully cleaner, more efficient and less fragile NIO transport
implementation.
Motivation:
394a1b3485 introduced a hard dependency on GLIBC 2.12 which was not the case before. This had the effect of not be able to use the native epoll transports on platforms which ship with earlier versions of GLIBC.
To make things a backward compatible as possible we should not introduce such changes in a bugfix release.
Special thanks to @weissi with all the help to fix this.
Modifications:
- Use syscalls directly to remove dependency on GLIBC 2.12
- Make code consistent that needs newer GLIBC versions
- Adjust scattering read test to only run if recvmmsg syscall is supported
- Cleanup pom.xml as some stuff is not needed anymore after using syscalls.
Result:
Fixes https://github.com/netty/netty/issues/9758.
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"));
Motivation
The recently-introduced event loop scheduling hooks can be exploited by
the epoll transport to avoid waking the event loop when scheduling
future tasks if there is a timer already set to wake up sooner.
There is also a "default" timeout which will wake the event
loop after 1 second if there are no pending future tasks. The
performance impact of these wakeups themselves is likely negligible but
there's significant overhead in having to re-arm the timer every time
the event loop goes to sleep (see #7816). It's not 100% clear why this
timeout was there originally but we're sure it's no longer needed.
Modification
Combine the existing volatile wakenUp and non-volatile prevDeadlineNanos
fields into a single AtomicLong that stores the next scheduled wakeup
time while the event loop is in epoll_wait, and is -1 while it is awake.
Use this as a guard to debounce wakeups from both immediate scheduled
tasks and future scheduled tasks, the latter using the new
before/afterScheduledTaskSubmitted overrides and based on whether the
new deadline occurs prior to an already-scheduled timer.
A similar optimization was already added to NioEventLoop, but it still
uses two separate volatiles. We should consider similar streamlining of
that in a future update.
Result
Fewer event loop wakeups when scheduling future tasks, greatly reduced
overhead when no future tasks are scheduled.
Motivation:
There is a goto statement above the current position of initialize dynamicMethods, and dynamicMethods is used after the goto which might cause undefined behavior.
Modifications:
Initialize dynamicMehtods at the top.
Result:
No more undefined behavior.
Motivation
This is a vestige that was removed in the original PR #9535 before it
was reverted, but we missed it when re-applying in #9586.
It means there is a possible race condition because a wakeup event could
be missed while shutting down, but the consequences aren't serious since
there's a 1 second safeguard timeout when waiting for it.
Modification
Remove call to epollWaitNow() in EpollEventLoop#closeAll()
Result
Cleanup redundant code, avoid shutdown delay race condition
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
Motivation:
The build script for the module Netty Transport Native Epoll can cause
intermittent build break due to broken pipe by /usr/bin/ldd. This issue
likely to occur on a build environment with multiple processors.
Modifications:
The root cause is that the consumer head command finishes earlier the
producer ldd command. Buffering the outputs of the ldd command by an
intermediate tail command avoids the broken pipe.
Result:
A build on multiple processors can finish successfully.
Signed-off-by: Tatsushi Inagaki <e29253@jp.ibm.com>
