Motivation:
We wish to separate these two into clearer write/read interfaces.
In particular, we don't want to be able to add listeners to promises, because it makes it easy to add them out of order.
We can't prevent it entirely, because any promise can be freely converted to a future where listeners can be added.
We can, however, discourage this in the API.
Modification:
The Promise interface no longer extends the Future interface.
Numerous changes to make the project compile and its tests run.
Result:
Clearer separation of concerns in the code.
Motivation:
Usually the outbound operation should start at the "current" ChanneöHandlercontext which was often not the case
Modifications:
Use the ChannelHandlerContext for closing the connection
Result:
Start the operation on the right position of the pipeline
Motivation:
The expression "not is success" can mean that either the future failed, or it has not yet completed.
However, many places where such an expression is used is expecting the future to have completed.
Specifically, they are expecting to be able to call `cause()` on the future.
It is both more correct, and semantically clearer, to call `isFailed()` instead of `!isSuccess()`.
Modification:
Change all places that used `!isSuccess()` to mean that the future had failed, to use `isFailed()`.
A few places are relying on `isSuccess()` returning `false` for _incomplete_ futures, and these places have been left unchanged.
Result:
Clearer code, with potentially fewer latent bugs.
* Remove deprecated Channel*Handler* classes
Motivation:
There is no need to keep the older adapter and duplex classes around.
Modifications:
- Remove old adapter and duplex classes
- Adjust javadocs
Result:
Cleanup
* Address nit
Motivation:
We should just add `executor()` to the `ChannelOutboundInvoker` interface and override this method in `Channel` to return `EventLoop`.
Modifications:
- Add `executor()` method to `ChannelOutboundInvoker`
- Let `Channel` override this method and return `EventLoop`.
- Adjust all usages of `eventLoop()`
- Add some default implementations
Result:
API cleanup
Motivation:
Since most futures in Netty are of the `Void` type, methods like `getNow()` and `cause()` cannot distinguish if the future has finished or not.
This can cause data race bugs which, in the case of `Void` futures, can be silent.
Modification:
The methods `getNow()` and `cause()` now throw an `IllegalStateException` if the future has not yet completed.
Most use of these methods are inside listeners, and so are not impacted.
One place in `AbstractBootstrap` was doing a racy read and has been adjusted.
Result:
Data race bugs around `getNow()` and `cause()` are no longer silent.
Motivation:
The generics for the existing futures, promises, and listeners are too complicated.
This complication comes from the existence of `ChannelPromise` and `ChannelFuture`, which forces listeners to care about the particular _type_ of future being listened on.
Modification:
* Add a `FutureContextListener` which can take a context object as an additional argument. This allows our listeners to have the channel piped through to them, so they don't need to rely on the `ChannelFuture.channel()` method.
* Make the `FutureListener`, along with the `FutureContextListener` sibling, the default listener API, retiring the `GenericFutureListener` since we no longer need to abstract over the type of the future.
* Change all uses of `ChannelPromise` to `Promise<Void>`.
* Change all uses of `ChannelFuture` to `Future<Void>`.
* Change all uses of `GenericFutureListener` to either `FutureListener` or `FutureContextListener` as needed.
* Remove `ChannelFutureListener` and `GenericFutureListener`.
* Introduce a `ChannelFutureListeners` enum to house the constants that previously lived in `ChannelFutureListener`. These constants now implement `FutureContextListener` and take the `Channel` as a context.
* Remove `ChannelPromise` and `ChannelFuture` — all usages now rely on the plain `Future` and `Promise` APIs.
* Add static factory methods to `DefaultPromise` that allow us to create promises that are initialised as successful or failed.
* Remove `CompleteFuture`, `SucceededFuture`, `FailedFuture`, `CompleteChannelFuture`, `SucceededChannelFuture`, and `FailedChannelFuture`.
* Remove `ChannelPromiseNotifier`.
Result:
Cleaner generics and more straight forward code.
Motivation:
This fixes a bug that would result in an `io.netty.channel.unix.Errors$NativeIoException: connectx(..) failed: Address family not supported by protocol family` error.
This happens when the connecting socket is configured to use IPv6 but the address being connected to is IPv4.
This can occur because, for instance, Netty and `InetAddress.getLoopbackAddress()` have different preferences for IPv6 vs. IPv4.
Modification:
Pass the correct ipv6 or ipv4 flags to connectx, depending on whether the socket was created for AF_INET or AF_INET6, rather than relying on the IP version of the destination address.
Result:
No more issue with TCP FastOpen on MacOS when using addresses of the "wrong" IP version.
Motivation:
The MacOS-specific `connectx(2)` system call make it possible to establish client-side connections with TCP FastOpen.
Modification:
Add support for TCP FastOpen to the KQueue transport, and add the `connectx(2)` system call to `BsdSocket`.
Result:
It's now possible to use TCP FastOpen when initiating connections on MacOS.
Bootstrap methods now return Future<Channel> instead of ChannelFuture
Motivation:
In #8516 it was proposed to at some point remove the specialised ChannelFuture and ChannelPromise.
Or at least make them not extend Future and Promise, respectively.
One pain point encountered in this discussion is the need to get access to the channel object after it has been initialised, but without waiting for the channel registration to propagate through the pipeline.
Modification:
Add a Bootstrap.createUnregistered method, which will return a Channel directly.
All other Bootstrap methods that previously returned ChannelFuture now return Future<Channel>
Result:
It's now possible to obtain an initialised but unregistered channel from a bootstrap, without blocking.
And the other bootstrap methods now only release their channels through the result of their futures, preventing racy access to the channels.
Motivation:
The TLS handshake must be able to finish on its own, without being driven by outside read calls.
This is currently not the case when TCP FastOpen is enabled.
Reads must be permitted and marked as pending, even when a channel is not active.
This is important because, with TCP FastOpen, the handshake processing of a TLS connection will start
before the connection has been established -- before the process of connecting has even been started.
The SslHandler on the client side will add the Client Hello message to the ChannelOutboundBuffer, then
issue a `ctx.read` call for the anticipated Server Hello response, and then flush the Client Hello
message which, in the case of TCP FastOpen, will cause the TCP connection to be established.
In this transaction, it is important that the `ctx.read` call is not ignored since, if auto-read is
turned off, this could delay or even prevent the Server Hello message from being processed, causing
the server-side handshake to time out.
Modification:
Attach a listener to the SslHandler.handshakeFuture in the EchoClient, that will call ctx.read.
Result:
The SocketSslEchoTest now tests that the SslHandler can finish handshakes on its own, without being driven by 3rd party ctx.read calls.
The various channel implementations have been updated to comply with this behaviour.
Motivation:
We did migrate all these modules to junit5 before but missed a few usages of junit4
Modifications:
Replace all junit4 imports by junit5 apis
Result:
Part of https://github.com/netty/netty/issues/10757
Motivation:
There are use cases when Unix domain datagram sockets are needed for communication.
This PR adds such support for Epoll/KQueue.
Modification:
- Expose Channel, Config and Packet interfaces/classes for Unix domain datagram sockets.
All interfaces/classes are in `transport-native-unix-common` module in order to be available
for KQueue and Epoll implementations
- Add JNI code for Unix domain datagram sockets
- Refactor `DatagramUnicastTest` so that it can be used for testing also Unix domain datagram sockets
- Add Unix domain datagram sockets implementation for KQueue transport
- Add Unix domain datagram sockets implementation for Epoll transport
Result:
Fixes#6737
Motivation:
Sometime in the past we introduced the concept of Void*Promise. As it turned out this was not a good idea at all as basically each handler in the pipeline need to be very careful to correctly handle this. We should better just remove this "optimization".
Modifications:
- Remove Void*Promise and all the related APIs
- Remove tests which were related to Void*Promise
Result:
Less error-prone API
Motivation:
JUnit 5 is more expressive, extensible, and composable in many ways, and it's better able to run tests in parallel.
Modifications:
Use JUnit5 in tests
Result:
Related to https://github.com/netty/netty/issues/10757
Motivation:
Let's also build on windows during PR validation
Modifications:
Add build on windows during PR
Result:
Validate that all also pass on windows
Motivation:
`PlatformDependent#normalizedOs()` already caches normalized variant of
the value of `os.name` system property. Instead of inconsistently
normalizing it in every case, use the utility method.
Modifications:
- `PlatformDependent`: `isWindows0()` and `isOsx0()` use `NORMALIZED_OS`;
- `PlatformDependent#normalizeOs(String)` define `darwin` as `osx`;
- `OpenSsl#loadTcNative()` does not require `equalsIgnoreCase` bcz `os`
is already normalized;
- Epoll and KQueue: `Native#loadNativeLibrary()` use `normalizedOs()`;
- Use consistent `Locale.US` for lower case conversion of `os.name`;
- `MacOSDnsServerAddressStreamProvider#loadNativeLibrary()` uses
`PlatformDependent.isOsx()`;
Result:
Consistent approach for `os.name` parsing.
Motivation:
SslHandler owns the responsibility to flush non-application data
(e.g. handshake, renegotiation, etc.) to the socket. However when
TCP Fast Open is supported but the client_hello cannot be written
in the SYN the client_hello may not always be flushed. SslHandler
may not wrap/flush previously written/flushed data in the event
it was not able to be wrapped due to NEED_UNWRAP state being
encountered in wrap (e.g. peer initiated renegotiation).
Modifications:
- SslHandler to flush in channelActive() if TFO is enabled and
the client_hello cannot be written in the SYN.
- SslHandler to wrap application data after non-application data
wrap and handshake status is FINISHED.
- SocketSslEchoTest only flushes when writes are done, and waits
for the handshake to complete before writing.
Result:
SslHandler flushes handshake data for TFO, and previously flushed
application data after peer initiated renegotiation finishes.
Motivation:
#10995
when `io.netty.channel.unix.Socket` is ipv6 and join a multicast group with ipv4 address will cause `io.netty.channel.ChannelException: setsockopt() failed: Invalid argument` (at least in `Linux centos.dev 4.18.0-240.10.1.el8_3.x86_64 #1 SMP Mon Jan 18 17:05:51 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux`)
Modification:
check if target group address is ipv6 before call `io.netty.channel.epoll.LinuxSocket#joinGroup(java.net.InetAddress, java.net.NetworkInterface, java.net.InetAddress)`
I'm not sure if this modification is currect, but i checked source code of java NIO
```
Java_sun_nio_ch_Net_canJoin6WithIPv4Group0(JNIEnv* env, jclass cl)
{
#if defined(__APPLE__)
/* IPV6_ADD_MEMBERSHIP can be used to join IPv4 multicast groups */
return JNI_TRUE;
#else
/* IPV6_ADD_MEMBERSHIP cannot be used to join IPv4 multicast groups */
return JNI_FALSE;
#endif
}
```
seems ipv6 address can't join ipv4 group except osx
Result:
test on `Linux 3.10.0-514.el7.x86_64 #1 SMP Tue Nov 22 16:42:41 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux` exception ` setsockopt() failed: Invalid argument` has fixed
Fixes#10995
Support TCP Fast Open for clients and make SslHandler take advantage
Motivation:
- TCP Fast Open allow us to send a small amount of data along side the initial SYN packet when establishing a TCP connection.
- The TLS Client Hello packet is small enough to fit in there, and is also idempotent (another requirement for using TCP Fast Open), so if we can save a round-trip when establishing TLS connections when using TFO.
Modification:
- Add support for client-side TCP Fast Open for Epoll, and also lowers the Linux kernel version requirements to 3.6.
- When adding the SslHandler to a pipeline, if TCP Fast Open is enabled for the channel (and the channel is not already active) then start the handshake early by writing it to the outbound buffer.
- An important detail to note here, is that the outbound buffer is not flushed at this point, like it would for normal handshakes. The flushing happens later as part of establishing the TCP connection.
Result:
- It is now possible for clients (on epoll) to open connections with TCP Fast Open.
- The SslHandler automatically detects when this is the case, and now send its Client Hello message as part of the initial data in the TCP Fast Open flow when available, saving a round-trip when establishing TLS connections.
Co-authored-by: Colin Godsey <crgodsey@gmail.com>
Motivation:
File.createTempFile(String, String)` will create a temporary file in the system temporary directory if the 'java.io.tmpdir'. The permissions on that file utilize the umask. In a majority of cases, this means that the file that java creates has the permissions: `-rw-r--r--`, thus, any other local user on that system can read the contents of that file.
This can be a security concern if any sensitive data is stored in this file.
This was reported by Jonathan Leitschuh <jonathan.leitschuh@gmail.com> as a security problem.
Modifications:
Use Files.createTempFile(...) which will use safe-defaults when running on java 7 and later. If running on java 6 there isnt much we can do, which is fair enough as java 6 shouldnt be considered "safe" anyway.
Result:
Create temporary files with sane permissions by default.
Motivation:
The testWriteAfterShutdownOutputNoWritabilityChange() failed a few times on the CI randomly. Let's skip it for now while we investigate and see if there is anything we can do to make the test less flaky on the CI.
Modifications:
Add @Ignore on the testWriteAfterShutdownOutputNoWritabilityChange method
Result:
Less flaky CI
Motivation:
https in xmlns URIs does not work and will let the maven release plugin fail:
```
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1.779 s
[INFO] Finished at: 2020-11-10T07:45:21Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-release-plugin:2.5.3:prepare (default-cli) on project netty-parent: Execution default-cli of goal org.apache.maven.plugins:maven-release-plugin:2.5.3:prepare failed: The namespace xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" could not be added as a namespace to "project": The namespace prefix "xsi" collides with an additional namespace declared by the element -> [Help 1]
[ERROR]
```
See also https://issues.apache.org/jira/browse/HBASE-24014.
Modifications:
Use http for xmlns
Result:
Be able to use maven release plugin
Motivation:
JUnit 5 is the new hotness. It's more expressive, extensible, and composable in many ways, and it's better able to run tests in parallel. But most importantly, it's able to directly run JUnit 4 tests.
This means we can update and start using JUnit 5 without touching any of our existing tests.
I'm also introducing a dependency on assertj-core, which is like hamcrest, but arguably has a nicer and more discoverable API.
Modification:
Add the JUnit 5 and assertj-core dependencies, without converting any tests at time time.
Result:
All our tests are now executed through the JUnit 5 Vintage Engine.
Also, the JUnit 5 test APIs are available, and any JUnit 5 tests that are added from now on will also be executed.
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:
junit deprecated Assert.assertThat(...)
Modifications:
Use MatcherAssert.assertThat(...) as replacement for deprecated method
Result:
Less deprecation warnings
Motivation:
Java 16 will come around eventually anyway, and this makes it easier for people to experiment with Early Access builds.
Modification:
- Added Maven profiles for JDK 16 to relevant pom files.
- Removed the `--add-exports java.base/sun.security.x509=ALL-UNNAMED` argument when running tests; we've not needed it since the Java11-as-baseline PR landed.
Result:
Netty now builds on JDK 16 pre-releases (provided they've not broken compatibility in some way).
Raise the Netty 5 minimum required Java version to Java 11.
Motivation:
Java 11 has been out for some time, and Netty 5 is still some ways out.
There are also many good features in Java 11 that we wish to use, such as VarHandles, var-keyword, and the module system.
There is no reason for Netty 5 to not require Java 11, since Netty 4.x will still be supported for the time being.
Modification:
Remove everything in the pom files related to Java versions older than Java 11.
Remove the animal-sniffer plug-in and rely on the `--release` compiler flag instead.
Remove docker files related to Java versions older than Java 11.
Remove the copied SCTP APIs -- we should test this commit independently on Windows.
Remove the OpenJdkSelfSignedCertGenerator.java file and just always use Bouncy Castle for generating self-signed certificates for testing.
Make netty-testsuite tests pass by including Bouncy Castle as a test dependency, so we're able to generate our self-signed certificate.
Result:
Java 11 is now the minimum required Java version.
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:
JDK15 is about to be released as GA, we should ensure netty works and builds on it. SSLSession#getPeerCertificateChain() throws UnsupportedOperationException in JDK15 and later as it was deprecated before and people should use SSLSession#getPeerCertificates(). We need to account for that in our tests
Modifications:
- Catch UnsupportedOperationException in our testsuite and ignore it when on JDK15+ while rethrowing it otherwise.
Result:
Testsuite passes on JDK15+
Motivation:
java.io.File.listFiles() may return null and cause a unexpected NPE.
Modification:
Add a null check for variable files. And if variable files is null, the compressHeapDumps method will just return after logging a error message.
Result:
Fix the potential NPE.
Motivation:
In next major version of netty users should use ChannelHandler everywhere. We should ensure we do the same
Modifications:
Replace usage of deprecated classes / interfaces with ChannelHandler
Result:
Use non-deprecated code
Motivation
Per javadoc in 4.1.x SimpleChannelInboundHandler:
"Please keep in mind that channelRead0(ChannelHandlerContext, I) will be
renamed to messageReceived(ChannelHandlerContext, I) in 5.0."
Modifications
Rename aforementioned method and all references/overrides.
Result
Method is renamed.
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.
This is a port of https://github.com/netty/netty/pull/9616
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()
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:
We should also use sendmmsg on connected channels whenever possible to reduce the overhead of syscalls.
Modifications:
No matter if the channel is connected or not try to use sendmmsg when supported to reduce the overhead of syscalls
Result:
Better performance on connected UDP channels due less syscalls
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:
At the moment it is quite easy to hit reentrance issues when you have multiple handlers in the pipeline and each of the handlers does not correctly protect against these. To make it easier for the user we should try to protect from these. The issue is usually if and inbound event will trigger and outbound event and this outbound event then against triggeres an inbound event. This may result in having methods in a ChannelHandler re-enter some method and so state can be corrupted or messages be re-ordered.
Modifications:
- Keep track of inbound / outbound operations in DefaultChannelHandlerContext and if reentrancy is detected break it by scheduling the action on the EventLoop. This will then be picked up once the method returns and so the reentrancy is broken up.
- Adjust tests which made strange assumptions about execution order
Result:
No more reentrancy of handlers possible.
Motivation:
If all we need is the FileChannel we should better use RandomAccessFile as FileInputStream and FileOutputStream use a finalizer.
Modifications:
Replace FileInputStream and FileOutputStream with RandomAccessFile when possible.
Result:
Fixes https://github.com/netty/netty/issues/8078.
Motivation:
We did not have support for enable / disable loopback mode in our native epoll transport and also missed the implemention to access the configured interface.
Modifications:
Add implementation and adjust test to cover it
Result:
More complete multicast support with native epoll transport
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:
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>
Motivation:
86dd388637 reverted the usage of IPv6 Multicast test. This commit makes the whole multicast testing a lot more robust by selecting the correct interface in any case and also reverts the `@Ignore`
Modifications:
- More robust multicast testing by selecting the right NetworkInterface
- Remove the `@Ignore` again for the IPv6 test
Result:
More robust multicast testing
Motivation:
The multicast ipv6 test fails on some systems. As I just added it let me ignore it for now while investigating.
Modifications:
Add @ignore
Result:
Stable testsuite while investigate
Motivation:
We currently only cover ipv4 multicast in the testsuite but we should also have tests for ipv6.
Modifications:
- Add test for ipv6
- Ensure we only try to run multicast test for ipv4 / ipv6 if the loopback interface supports it.
Result:
Better test coverage
Motivation:
We should not throw check exceptions when the user calls sync*() but should better wrap it in a CompletionException to make it easier for people to reason about what happens.
Modifications:
- Change sync*() to throw CompletionException
- Adjust tests
- Add some more tests
Result:
Fixes https://github.com/netty/netty/issues/8521.
