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:
ddebc1027d missed to make Errors.throwConnectException(...) public
Modifications:
Make method public
Result:
Be able to use Errors.throwConnectException(...) from other module
Motivation:
During the last few month we did develop an io_uring based transport which shows very promising performance numbers. To give it more time to bake we will develop it outside of netty in an "incubator" module which will make it clear to users what to expect and also allow us to seperate its release cycle. While the implementation of it is very self contained there are few small adjustments that need to be made in netty itself to allow us to reuse code.
Modifications:
- AbstractChannel: Add method which can be used when a write fails and remove final from one method
- IovArray: Allow to create an IovArray from a ByteBuf instance
- FileDescriptor: Allow to reuse mark close logic via sub-class
Result:
Be able to reuse netty core classes in io_uring incubator repository
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:
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:
In some situations we could have end up calling some functions with NULL parameters which in this case could lead to undefined behavior. All of this would have happened during loading of the native lib.
Modifications:
Add NULL check as guards and return early
Result:
Fix some possible undefined behavior
Motivation:
The feature test macros did not work as expected on MacOS, so we ended up compiling for the GNU
variant which resulted in compilation errors.
Modification:
Add `__APPLE__` as another indicator to use the XSI variant of `strerror_r`.
Result:
The project now once again compiles on MacOS.
Motivation:
We previously relied on `strerror`, but this function is unfortunately not thread-safe.
Modification:
The use of `strerror` has been changed to `strerror_r`, which is thread-safe.
This function has a more complicated API, and has portability concerns that needs to be handled.
This accounts for the relatively large increase in lines of code.
Result:
Error messages from JNI are now always generated in a thread-safe way.
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:
Modern versions of FreeBSD define IP_RECVORIGDSTADDR, but don't define
SOL_IP. This causes the build to fail.
Modifications:
The equivalent to SOL_IP on FreeBSD is IPPROTO_IP. Define SOL_IP as that
if SOL_IP is not defined and IPPROTO_IP is.
Result:
This allows a successful build on FreeBSD
Motivation:
On MacOS it is not really good enough to check /etc/resolv.conf to determine the nameservers to use. We should retrieve the nameservers using the same way as mDNSResponser and chromium does by doing a JNI call.
Modifications:
Add MacOSDnsServerAddressStreamProvider and testcase
Result:
Use correct nameservers by default on MacOS.
Motivation:
We would like to enable Netty also on a big endian platform such as
s390_64. We need to fix a function which assumes that the target
platform is little endian.
Modifications:
Modify netty_unix_socket_accept() to write an address length as jbyte
instead of jsize.
Result:
Netty can be enabled on a big endian platform.
Signed-off-by: Tatsushi Inagaki <e29253@jp.ibm.com>
Motivation:
At the moment we not consistently (and also not correctly) free allocated native memory in all cases during loading the JNI library. This can lead to native memory leaks in the unlikely case of failure while trying to load the library.
Beside this we also not always correctly handle the case when a new java object can not be created in native code because of out of memory.
Modification:
- Copy some macros from netty-tcnative to be able to handle errors in a more easy fashion
- Correctly account for New* functions to return NULL
- Share code
Result:
More robust and clean JNI code
Motivation:
291f80733a introduced a change to use a byte[] to construct the InetAddress when receiving datagram messages to reduce the overhead. Unfortunally it introduced a regression when handling IPv6-mapped-IPv4 addresses and so produced an IndexOutOfBoundsException when trying to fill the byte[] in native code.
Modifications:
- Correctly use the offset on the pointer of the address.
- Add testcase
- Make tests more robust and include more details when the test fails
Result:
No more IndexOutOfBoundsException
Motivation:
394a1b3485 introduced the possibility to use recvmmsg(...) but did not correctly handle ipv6 mapped ip4 addresses to make it consistent with other transports.
Modifications:
- Correctly handle ipv6 mapped ipv4 addresses by only copy over the relevant bytes
- Small improvement on how to detect ipv6 mapped ipv4 addresses by using memcmp and not byte by byte compare
- Adjust test to cover this bug
Result:
Correctly handle ipv6 mapped ipv4 addresses
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.
Motivation:
At the moment we use the String representation of the IP to create the DatagramSocketAddress. This is not for free and we should better use the byte[] directly to reduce the overhead of parsing the String (and creating it in the first place)
Modifications:
Directly use byte[] as input for the DatagramSocketAddress
Result:
Less overhead when using Datagrams with native transports
Motivation:
Provide epoll/native multicast to support high load multicast users (we are using it for a high load telecomm app at my day job).
Modification:
Added support for source specific and any source multicast for epoll transport. Some caveats: no support for disabling loop back mode, retrieval of interface and block operation, all of which tend to be less frequently used.
Result:
Provides epoll transport multicast for common use cases.
Co-authored-by: Norman Maurer <norman_maurer@apple.com>
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.
Motivation:
When kevent(...) returns with EINTR we do not correctly decrement the timespec
structure contents to account for the time duration. This may lead to negative
values for tv_nsec which will result in an EINVAL and raise an IOException to
the event loop selection loop.
Modifications:
Correctly calculate new timeoutTs when EINTR is detected
Result:
Fixes#9013.
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.
* 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.
Motivation:
netty_unix_socket attempts to use nettyClassName in an error message, but previously freed the memory. We should wait to free the memory until after we use it.
Modifications:
- Free nettyClassName after using it in snprintf
Result:
More useful error message.
Motivation:
b818852cdb broke support for shading the native libraries in netty as it missed to respect the package prefix that is used when shading.
Modifications:
Correctly respect package prefix for constructor argument and include the used classname when logging that we could not find the constructor.
Result:
Be able to shade native libraries of netty again.
Motivation:
b818852cdb added support for IP_RECVORIGDSTADDR but did not include any #ifdef statements to ensure its usable at all (which is not the case on MacOS).
Modifications:
Add #ifdef statements to check for IP_RECVORIGDSTADDR.
Result:
Compilation works again on MacOS.
Motivation:
DatagramPacket.recipient() doesn't return the actual destination IP, but the IP the app is bound to.
Modification:
- IP_RECVORIGDSTADDR option is enabled for UDP sockets, which allows retrieval of ancillary information containing the original recipient.
- _recvFrom(...) function from transport-native-unix-common/src/main/c/netty_unix_socket.c is modified such that if IP_RECVORIGDSTADDR is set, recvmsg is used instead of recvfrom; enabling the retrieval of the original recipient.
- DatagramSocketAddress also contains a 'local' address, representing the recipient.
- EpollDatagramChannel is updated to return the retrieved recipient address instead of the address the channel is bound to.
Result:
Fixes#4950.
Motivation:
If close(...) reports EINTR there is nothing sane we can do so it makes no sense to even report it. See also:
https://github.com/apple/swift-nio/pull/217
Modifications:
Just ignore EINTR when calling close(...)
Result:
Less noise in the logs.
Motivation:
We currently not check if the buffer has a memory address and just assume this is the case if the nioBufferCount() == 1.
Modifications:
- Check hasMemoryAddress() before trying to access it.
- Add unit case.
Result:
More correct and robust code. Related to [#7752].
Motivation:
IovArray implements MessageProcessor, and the processMessage method will continue to be called during iteration until it returns true. A recent commit b215794de3 changed the return value to only return true if any component of a CompositeByteBuf was added as a result of the method call. However this results in the iteration continuing, and potentially subsequent smaller buffers maybe added, which will result in out of order writes and generally corrupts data.
Modifications:
- IovArray#add should return false so that the MessageProcessor#processMessage will stop iterating.
Result:
Native transports which use IovArray will not corrupt data during gathering writes of CompositeByteBuf objects.
Motivation:
FileDescriptor#writev calls JNI code, and that JNI code dereferences a NULL pointer which crashes the application. This occurs when writing a single CompositeByteBuf object with more than one component.
Modifications:
- Initialize the iovec iterator properly to avoid the core dump
- Fix the array length calculation if we aren't able to fit all the ByteBuffer objects in the iovec array
Result:
No more core dump.
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.
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].
Motivation:
Exception handling is nicer when a more specific Exception is thrown
Modification:
Add a static reference for ENOENT, and throw FNFE if it is returned
Result:
More precise exception handling
Motivation:
To better isolate OS system calls we should not call getsockopt directly but use our netty_unix_socket_getOption0 function. See is a followup of f115bf5.
Modifications:
Export netty_unix_socket_getOption0 by declaring it in the header file and use it
Result:
Better isolation of system calls.
Motivation:
If a user calls EpollSocketChannelConfig.getOptions() and TCP_FASTOPEN_CONNECT is not supported we throw an exception.
Modifications:
- Just return 0 if ENOPROTOOPT is set.
- Add testcase
Result:
getOptions() works as epxected.
Motivation:
We tried to set IPV6 opts on an ipv4 only system and so failed to set / get the traffic opts. This resulted in a test-error when trying to compile netty on ipv4 only systems.
Modifications:
Use the correct opts depending on if the system is ipv4 only or not.
Result:
Be able to build and use on ipv4 only systems.
Motivation:
We should try to bind to an ipv6 only socket before we enable ipv6 support in the native transport as it may not work due setup of the platform.
Modifications:
Try to bind to ::1 use IPV6 later on if this works
Result:
Fixes [#7021].
Motivation:
At the moment we try to load the library using multiple names which includes names using - but also _ . We should just use _ all the time.
Modifications:
Replace - with _
Result:
Fixes [#7069]
Motivation:
We did not correctly handle connect() and disconnect() in EpollDatagramChannel / KQueueDatagramChannel and so the behavior was different compared to NioDatagramChannel.
Modifications:
- Correct implement connect and disconnect methods
- Share connect and related code
- Add tests
Result:
EpollDatagramChannel / KQueueDatagramChannel also supports correctly connect() and disconnect() methods.
Motivation:
In some environments, IPv4 may be disabled (at a kernel level).
Google has such an environment for testing v4 -> v6 transition
paths. This give confidence that code is v6 ready.
Modifications:
Change native socket code to ignore failures of trying to enter
dual stack mode. This change has been made to Google's internal
JDK, and will/should be upstreamed to OpenJDK eventually.
Results:
Netty works in IPv6 only environments
Fixes: #6993
fix not execute unit test in transport-native-unix-common-tests module
Motivation:
- Commit 047da11 introduced an bug for still copy byteBuf for composed of n(n <= IOV_MAX) NIO direct buffers
- Commit 3c4dfed add UnixChannelUtilTest in transport-native-unix-common-tests module, but not execute in maven compile
as issue #6825, #6870
Modifications:
- modified UnixChannelUtil#isBufferCopyNeededForWrite(ByteBuf), and UnixChannelUtilTest
- move UnixChannelUtilTest into transport-native-unix-common module, and add packet scope method UnixChannelUtil#isBufferCopyNeededForWrite(ByteBuf, int)
Result:
- no copy byteBuf for composed of n(n <= IOV_MAX) NIO direct buffers
- auto execute unit tests in UnixChannelUtilTest and it is easier to mock IOV_MAX
Motivation:
Enable static linking for Java 8. These commits are the same as those introduced to netty tcnative. The goal is to allow lots of JNI libraries to be statically linked together without having conflict `JNI_OnLoad` methods.
Modification:
* add JNI_OnLoad suffixes to enable static linking
* Add static names to the list of libraries that try to be loaded
* Enable compiling with JNI 1.8
* Sort includes
Result:
Enable statically linked JNI code.
Motivation:
Google requires stricter compilation by adding -Werror and enabling many other warnings.
Modification:
* fix warning caused by -Wmissing-braces
* Use the address of `sendmmsg` rather than the function itself when
checking for presence. This resovles the warning caused by
`-Wpointer-bool-conversion`.
More detail:
When compiling on Linux, `sendmmsg` is always present, so the
function is always nonnull. When compiling elsewhere, the
function is defined as `__attribute__((weak))` which means it
may be absent at link time. This is controlled by
`IO_NETTY_SENDMMSG_NOT_FOUND`, which is off by default.
The reason for the error is due to the risk of accidentally not
calling the function. By adding `&` before the function, there
is no ambiguity. (the result of the fn call cannot have its
address taken.)
* use != to check for sendmmsg
Result:
Easier compilation.
Motivation:
Commit 3c4dfed08a introduced a regression in handling buffers that have no memoryAddress.
Modifications:
Fix regression and also add unit tests.
Result:
It's possible again to write buffers without memory address.
Motivation:
We used strstr to find the path to the library, which fails if the library is contained in a directory that also matches the library name.
Modifications:
- Introduce netty_unix_util_strstr_last which will return a pointer which points to the last accourance and so not fails if the direct also matches the library name.
Result:
Be able to load the library in all cases.