Motivation:
In Netty 5 we wish to have a simpler, safe, future proof, and more consistent buffer API.
We developed such an API in the incubating buffer repository, and taking it through multiple rounds of review and adjustments.
This PR/commit bring the results of that work into the Netty 5 branch of the main Netty repository.
Modifications:
* `Buffer` is an interface, and all implementations are hidden behind it.
There is no longer an inheritance hierarchy of abstract classes and implementations.
* Reference counting is gone.
After a buffer has been allocated, calling `close` on it will deallocate it.
It is then up to users and integrators to ensure that the life-times of buffers are managed correctly.
This is usually not a problem as buffers tend to flow through the pipeline to be released after a terminal IO operation.
* Slice and duplicate methods are replaced with `split`.
By removing slices, duplicate, and reference counting, there is no longer a possibility that a buffer and/or its memory can be shared and accessible through multiple routes.
This solves the problem of data being accessed from multiple places in an uncoordinated way, and the problem of buffer memory being closed while being in use by some unsuspecting piece of code.
Some adjustments will have to be made to other APIs, idioms, and usages, since `split` is not always a replacement for `slice` in some use cases.
* The `split` has been added which allows memory to be shared among multiple buffers, but in non-overlapping regions.
When the memory regions don't overlap, it will not be possible for the different buffers to interfere with each other.
An internal, and completely transparent, reference counting system ensures that the backing memory is released once the last buffer view is closed.
* A Send API has been introduced that can be used to enforce (in the type system) the transfer of buffer ownership.
This is not expected to be used in the pipeline flow itself, but rather for other objects that wrap buffers and wish to avoid becoming "shared views" — the absence of "shared views" of memory is important for avoiding bugs in the absence of reference counting.
* A new BufferAllocator API, where the choice of implementation determines factors like on-/off-heap, pooling or not.
How access to the different allocators will be exposed to integrators will be decided later.
Perhaps they'll be directly accessible on the `ChannelHandlerContext`.
* The `PooledBufferAllocator` has been copied and modified to match the new allocator API.
This includes unifying its implementation that was previously split across on-heap and off-heap.
* The `PooledBufferAllocator` implementation has also been adjusted to allocate 4 MiB chunks by default, and a few changes have been made to the implementation to make a newly created, empty allocator use significantly less heap memory.
* A `Resource` interface has been added, which defines the life-cycle methods and the `send` method.
The `Buffer` interface extends this.
* Analogues for `ByteBufHolder` has been added in the `BufferHolder` and `BufferRef` classes.
* `ByteCursor` is added as a new way to iterate the data in buffers.
The byte cursor API is designed to be more JIT friendly than an iterator, or the existing `ByteProcessor` interface.
* `CompositeBuffer` no longer permit the same level of access to its internal components.
The composite buffer enforces its ownership of its components via the `Send` API, and the components can only be individually accessed with the `forEachReadable` and `forEachWritable` methods.
This keeps the API and behavioral differences between composite and non-composite buffers to a minimum.
* Two implementations of the `Buffer` interface are provided with the API: One based on `ByteBuffer`, and one based on `sun.misc.Unsafe`.
The `ByteBuffer` implementation is used by default.
More implementations can be loaded from the classpath via service loading.
The `MemorySegment` based implementation is left behind in the incubator repository.
* An extensive and highly parameterised test suite has been added, to ensure that all implementations have consistent and correct behaviour, regardless of their configuration or composition.
Result:
We have a new buffer API that is simpler, better tested, more consistent in behaviour, and safer by design, than the existing `ByteBuf` API.
The next legs of this journey will be about integrating this new API into Netty proper, and deprecate (and eventually remove) the `ByteBuf` API.
This fixes#11024, #8601, #8543, #8542, #8534, #3358, and #3306.
Motivation:
WeakOrderQueue would drop object that has been recycled, even when it has space for it.
WeakOrderQueue#add should check DefaultHandler.hasBeenRecycler field first
Modifications:
WeakOrderQueue test the DefaultHandler.hasBeenRecycler first
Result:
WeakOrderQueue would not drop object that has been recycled when there is space
Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Co-authored-by: Trustin Lee <t@motd.kr>
Motivation:
We need to use a GraalVM dependency which uses GPL2 + CE.
Modifications:
- Update all graalvm dependencies to new GAV which introduces a license change from GPL2 to GPL2 + CE
- This also required a small bump on the general version from 19.2 to 19.3, which should be fine as 19.3 is an official maintained LTS version, while 19.2 wasn't.
Result:
Fixes: #11398
Signed-off-by: Paulo Lopes <pmlopes@gmail.com>
Motivation:
This special case implementation of Promise / Future requires the implementations responsible for completing the promise to have knowledge of this class to provide value. It also requires that the implementations are able to provide intermediate status while the work is being done. Even throughout the core of Netty it is not really supported most of the times and so just brings more complexity without real gain.
Let's remove it completely which is better then only support it sometimes.
Modifications:
Remove Progressive* API
Result:
Code cleanup.... Fixes https://github.com/netty/netty/issues/8519
Motivation:
We didnt really have a good use-case for removeListener* and addListeners. Because of this we should just remove these methods and so make things simpler.
Modifications:
Remove methods
Result:
Cleanup
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:
1. The docs about the 'retun value' of the method `io.netty.util.internal.MathUtil#isOutOfBounds` is not correct.
2. The capacity parameter should be checked for overflowed case.
Modification:
1. Changed the doc to:
> @return {@code false} if the requested {@code index} and {@code length} will fit within {@code capacity}.
> {@code true} if this would result in an index out of bounds exception.
2. Improved the bounder checking logic.
Result:
Fixes#11279Fixes#11280
Motivation:
The current initialization of Slf4JLoggerFactory is not singleton.
Modification:
Use Slf4JLoggerFactory.INSTANCE to initialize Slf4JLoggerFactory.
Result:
The instance of Slf4JLoggerFactory became a singleton.
Motivation:
DefaultHostsFileEntriesResolver should provide all hosts file's entries for a hostname when
DnsNameResolver#resolveAll as opposed to the current implementation where only the first
entry is taken into consideration
Modification:
- Add DefaultHostsFileEntriesResolver#addresses to provide all hosts file's entries for a hostname
- Add HostsFileEntriesProvider to provide all hosts file's entries for a hostname and to keep
backwards compatibility for HostsFileEntries and HostsFileParser
- DnsNameResolver#resolveAll uses the new DefaultHostsFileEntriesResolver#addresses
- BlockHound configuration: replace HostsFileParser#parse with HostsFileEntriesProvider$ParserImpl#parse
as the latter does the parsing
- Add junit tests
Result:
Fixes#10834
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:
It turns out it is quite easy to cause a classloader deadlock in more recent java updates if you cause classloading while you are in native code. Because of this we should just workaround this issue by pre-load all the classes that needs to be accessed in the OnLoad function.
Modifications:
- Preload all classes that would otherwise be loaded by native OnLoad functions.
Result:
Workaround for https://github.com/netty/netty/issues/11209 and https://bugs.openjdk.java.net/browse/JDK-8266310
Motivation:
NullChecks resulting in a NullPointerException or IllegalArgumentException, numeric ranges (>0, >=0) checks, not empty strings/arrays checks must never be anonymous but with the parameter or variable name which is checked. They must be specific and should not be done with an "OR-Logic" (if a == null || b == null) throw new NullPointerEx.
Modifications:
* Add some checks to ObjectUtil not present today but utilized in the code.
* Add unit test for ObjectUtil
* Update commmons internal usage with ObjectUtil
Result:
All checks needed are present, subsequent changes of usage of ObjectUtil are possible.
Fixes for https://github.com/netty/netty/issues/11170
Motivation:
We are increasingly running in environments where Unsafe, setAccessible, etc. are not available.
When debug logging is enabled, we log a complete stack trace every time one of these initialisations fail.
Seeing these stack traces can cause people unnecessary concern.
For instance, people might have alerts that are triggered by a stack trace showing up in logs, regardless of its log level.
Modification:
We continue to print debug log messages on the result of our initialisations, but now we only include the full stack trace is _trace_ logging (or FINEST, or equivalent in whatever logging framework is configured) is enabled.
Result:
We now only log these initialisation stack traces when the lowest possible log level is enabled.
Fixes#7817
Motivation:
We had a bug in out DefaulThreadFactory as it always retrieved the ThreadGroup to used during creation time when now explicit ThreadGroup was given. This is problematic as the Thread may die and so the ThreadGroup is destroyed even tho the DefaultThreadFactory is still used.
This could produce exceptions like:
java.lang.IllegalThreadStateException
at java.lang.ThreadGroup.addUnstarted(ThreadGroup.java:867)
at java.lang.Thread.init(Thread.java:405)
at java.lang.Thread.init(Thread.java:349)
at java.lang.Thread.<init>(Thread.java:599)
at io.netty.util.concurrent.FastThreadLocalThread.<init>(FastThreadLocalThread.java:60)
at io.netty.util.concurrent.DefaultThreadFactory.newThread(DefaultThreadFactory.java:122)
at io.netty.util.concurrent.DefaultThreadFactory.newThread(DefaultThreadFactory.java:106)
at io.netty.util.concurrent.ThreadPerTaskExecutor.execute(ThreadPerTaskExecutor.java:32)
at io.netty.util.internal.ThreadExecutorMap$1.execute(ThreadExecutorMap.java:57)
at io.netty.util.concurrent.SingleThreadEventExecutor.doStartThread(SingleThreadEventExecutor.java:978)
at io.netty.util.concurrent.SingleThreadEventExecutor.startThread(SingleThreadEventExecutor.java:947)
at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:830)
at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:818)
at io.netty.channel.AbstractChannel$AbstractUnsafe.register(AbstractChannel.java:471)
at io.netty.channel.SingleThreadEventLoop.register(SingleThreadEventLoop.java:87)
at io.netty.channel.SingleThreadEventLoop.register(SingleThreadEventLoop.java:81)
at io.netty.channel.MultithreadEventLoopGroup.register(MultithreadEventLoopGroup.java:86)
at io.netty.bootstrap.AbstractBootstrap.initAndRegister(AbstractBootstrap.java:323)
at io.netty.bootstrap.AbstractBootstrap.doBind(AbstractBootstrap.java:272)
at io.netty.bootstrap.AbstractBootstrap.bind(AbstractBootstrap.java:239)
at io.netty.incubator.codec.quic.QuicTestUtils.newServer(QuicTestUtils.java:138)
at io.netty.incubator.codec.quic.QuicTestUtils.newServer(QuicTestUtils.java:143)
at io.netty.incubator.codec.quic.QuicTestUtils.newServer(QuicTestUtils.java:147)
at io.netty.incubator.codec.quic.QuicStreamFrameTest.testCloseHalfClosure(QuicStreamFrameTest.java:48)
at io.netty.incubator.codec.quic.QuicStreamFrameTest.testCloseHalfClosureUnidirectional(QuicStreamFrameTest.java:35)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:288)
at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:282)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at java.lang.Thread.run(Thread.java:748)
Modifications:
- If the user dont specify a ThreadGroup we will just pass null to the constructor of FastThreadLocalThread and so let it retrieve on creation time
- Adjust tests
Result:
Don't risk to see IllegalThreadStateExceptions.
Motivation:
The offsets were accidentally typed as int, where they should have been typed as long.
Modification:
Change type of offset arguments to PlatformDependent.put*(Object,int,?) from int to long.
Result:
It is now possible to use these methods to store to memory at absolute memory addresses.
Motivation:
These are necessary for creating a buffer implementation that uses Unsafe, and works generically for both on-heap and off-heap memory.
Modification:
PlatformDependent previously forced clients to decide if they are working on on-heap memory, or off-heap memory, by giving accessors distinct APIs for each.
What is added here, are generic accessors that work the same in either case.
Result:
We can now make an Unsafe-based buffer implementation that is agnostic to whether the memory is on- or off-heap.
Motivation:
Alignment handling was broken, and basically turned into a fixed offset into each allocation address regardless of its initial value, instead of ensuring that the allocated address is either aligned or bumped to the nearest alignment offset.
The brokenness of the alignment handling extended so far, that overlapping ByteBuf instances could even be created, as was seen in #11101.
Modification:
Instead of fixing the per-allocation pointer bump, we now ensure that 1) the minimum page size is a whole multiple of the alignment, and 2) the reference memory for each chunk is bumped to the nearest aligned address, and finally 3) ensured that the reservations are whole multiples of the alignment, thus ensuring that the next allocation automatically occurs from an aligned address.
Incidentally, (3) above comes for free because the reservations are in whole pages, and in (1) we ensured that pages are sized in whole multiples of the alignment.
In order to ensure that the memory for a chunk is aligned, we introduce some new PlatformDependent infrastructure.
The PlatformDependent.alignDirectBuffer will produce a slice of the given buffer, and the slice will have an address that is aligned.
This method is plainly available on ByteBuffer in Java 9 onwards, but for pre-9 we have to use Unsafe, which means it can fail and might not be available on all platforms.
Attempts to create a PooledByteBufAllocator that uses alignment, when this is not supported, will throw an exception.
Luckily, I think use of aligned allocations are rare.
Result:
Aligned pooled byte bufs now work correctly, and never have any overlap.
Fixes#11101
Motivation:
Add test to validate we are not affected by the regression introduced by fd8c1874b4
Modifications:
- Add unit test
Result:
Verify code works as expected
Motivation:
It is possible for two separate threads to race on recycling an object.
If this happens, the object might be added to a WeakOrderQueue when it shouldn't be.
The end result of this is that an object could be acquired multiple times, without a recycle in between.
Effectively, it ends up in circulation twice.
Modification:
We fix this by making the update to the lastRecycledId field of the handle, an atomic state transition.
Only the thread that "wins" the race and succeeds in their state transition will be allowed to recycle the object.
The others will bail out on their recycling.
We use weakCompareAndSet because we only need the atomicity guarantee, and the program order within each thread is sufficient.
Also, spurious failures just means we won't recycle that particular object, which is fine.
Result:
Objects no longer risk circulating twice due to a recycle race.
This fixes#10986
Motivation:
When etcResolver/hosts files are parsed, FileInputStream.read(...) is internally called by
- UnixResolverDnsServerAddressStreamProvider#parseEtcResolverSearchDomains
- UnixResolverDnsServerAddressStreamProvider#parseEtcResolverOptions
- HostsFileParser#parse
This will cause the error below when BlockHound is enabled
reactor.blockhound.BlockingOperationError: Blocking call! java.io.FileInputStream#readBytes
at java.io.FileInputStream.readBytes(FileInputStream.java)
at java.io.FileInputStream.read(FileInputStream.java:255)
Modifications:
- Add whitelist entries to BlockHound configuration
- Fix typos in UnixResolverDnsServerAddressStreamProvider
- Add tests
Result:
Fixes#11004
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:
We produced a lot of noise during loading native libraries as we always included the stacktrace if we could not load by one mechanism. We should better just not include the stacktrace in the debugging logging if one mechanism fails. We will log all the stacks anyway when all of the mechanisms fail.
Modifications:
Make logging less aggressive
Result:
Less confusing behaviour for the end-user
Motivation:
Internally UnixResolverDnsServerAddressStreamProvider#parse calls FileInputStream.read(...)
when parsing the etcResolverFiles.
This will cause the error below when BlockHound is enabled
reactor.blockhound.BlockingOperationError: Blocking call! java.io.FileInputStream#readBytes
at java.io.FileInputStream.readBytes(FileInputStream.java)
at java.io.FileInputStream.read(FileInputStream.java:255)
Modifications:
- Add whitelist entry to BlockHound configuration
- Add test
Result:
Fixes#10925
Motivation:
A race detector discovered a data race in GlobalEventExecutor present in
netty 4.1.51.Final:
```
Write of size 4 at 0x0000cea08774 by thread T103:
#0 io.netty.util.internal.DefaultPriorityQueue.poll()Lio/netty/util/internal/PriorityQueueNode; DefaultPriorityQueue.java:113
#1 io.netty.util.internal.DefaultPriorityQueue.poll()Ljava/lang/Object; DefaultPriorityQueue.java:31
#2 java.util.AbstractQueue.remove()Ljava/lang/Object; AbstractQueue.java:113
#3 io.netty.util.concurrent.AbstractScheduledEventExecutor.pollScheduledTask(J)Ljava/lang/Runnable; AbstractScheduledEventExecutor.java:133
#4 io.netty.util.concurrent.GlobalEventExecutor.fetchFromScheduledTaskQueue()V GlobalEventExecutor.java:119
#5 io.netty.util.concurrent.GlobalEventExecutor.takeTask()Ljava/lang/Runnable; GlobalEventExecutor.java:106
#6 io.netty.util.concurrent.GlobalEventExecutor$TaskRunner.run()V GlobalEventExecutor.java:240
#7 io.netty.util.internal.ThreadExecutorMap$2.run()V ThreadExecutorMap.java:74
#8 io.netty.util.concurrent.FastThreadLocalRunnable.run()V FastThreadLocalRunnable.java:30
#9 java.lang.Thread.run()V Thread.java:835
#10 (Generated Stub) <null>
Previous read of size 4 at 0x0000cea08774 by thread T110:
#0 io.netty.util.internal.DefaultPriorityQueue.size()I DefaultPriorityQueue.java:46
#1 io.netty.util.concurrent.GlobalEventExecutor$TaskRunner.run()V GlobalEventExecutor.java:263
#2 io.netty.util.internal.ThreadExecutorMap$2.run()V ThreadExecutorMap.java:74
#3 io.netty.util.concurrent.FastThreadLocalRunnable.run()V FastThreadLocalRunnable.java:30
#4 java.lang.Thread.run()V Thread.java:835
#5 (Generated Stub) <null>
```
The race is legit, but benign. To trigger it requires a TaskRunner to
begin exiting and set 'started' to false, more work to be scheduled
which starts a new TaskRunner, that work then needs to schedule
additional work which modifies 'scheduledTaskQueue', and then the
original TaskRunner checks 'scheduledTaskQueue'. But there is no danger
to this race as it can only produce a false negative in the condition
which causes the code to CAS 'started' which is thread-safe.
Modifications:
Delete problematic references to scheduledTaskQueue. The only way
scheduledTaskQueue could be modified since the last check is if another
TaskRunner is running, in which case the current TaskRunner doesn't
care.
Result:
Data-race free code, and a bit less code to boot.
Motivation:
I did not see any tangible advantage to the padding.
The only other field that was guarded was a rarely changed object reference to a BitSet.
Without the padding, there is also no longer any use of the inheritance hierarchy.
The padding was also using `long`, which would not necessarily prevent the JVM from fitting the aforementioned object reference in an alignment gap.
Modification:
Move all the fields into the InternalThreadLocalMap
Result:
Simpler code.
This resolves the discussion in https://github.com/netty/netty/issues/9284
Motivation:
We shouldnt catch Throwable in InternalLoggerFactory as this may hide OOME etc.
Modifications:
Only catch LinkageError and Exception
Result:
Fixes https://github.com/netty/netty/issues/10857
Motivation:
In #10630, field substitutions were introduced for NetUtil.LOCALHOST4, NetUtil.LOCALHOST6 and NetUtil.LOCALHOST fields. They were required to allow a native image be built with most of Netty (including NetUtil) initialized at build time.
The substitutions created in #10630 only define getters, so the 3 fields can only be read in a native image.
But when NetUtil is initialized at run-time (this is what happens in #10797), its static initialization block is executed, and this block writes to all 3 fields. As the substitutions do not provide any setters, field stores are not valid, and such builds fail.
Modifications:
- Add netty-testsuite-native-image-client-runtime-init Maven module that builds a native image deferring NetUtil class initialization till run-time; this module is used to demonstrate the problem and verify the problem is gone with the fix
- Add no-op setters to substitutions for NetUtil.LOCALHOST4, NetUtil.LOCALHOST6 and NetUtil.LOCALHOST
Result:
A native image initializing NetUtil at run-time builds successfully.
Fixes#10797
Motivation:
Internally SSLEngineImpl.wrap(...) may call FileInputStream.read(...).
This will cause the error below when BlockHound is enabled
reactor.blockhound.BlockingOperationError: Blocking call! java.io.FileInputStream#readBytes
at java.io.FileInputStream.readBytes(FileInputStream.java)
at java.io.FileInputStream.read(FileInputStream.java:255)
Modifications:
- Add whitelist entry to BlockHound configuration
- Add test
Result:
Fixes#10837
Motivation:
If Log4J2's `Filter` creates `Recycler.Stack` somehow, `Recycler.Stack()` will see uninitialized `Recycler.INITIAL_CAPACITY`. This has been raised originally in https://github.com/micrometer-metrics/micrometer/issues/2369.
Modification:
This PR changes to initialize `Recycler.INITIAL_CAPACITY` before invoking `InternalLogger.debug()` to avoid it.
Result:
Fixes the problem described in the "Motivation" section.
Motivation:
We should avoid lying with throws declarations whenever possible.
Modification:
Changed the code to instead directly throw Error, which seems to have been the intent.
Also, while we're here, convert its associated test to JUnit 5 and clean it up a bit.
Result:
Cleaner code.
Motivation:
GlobalEventExecutor.addTask was rightfully allowed to block by commit
09d38c8. However the same should have been done for
SingleThreadEventExecutor.addTask.
BlockHound is currently intercepting that call, and as a consequence,
it prevents SingleThreadEventExecutor from working properly, if addTask is
called from a thread that cannot block.
The interception is due to LinkedBlockingQueue.offer implementation,
which uses a ReentrantLock internally.
Modifications:
* Added one BlockHound exception to
io.netty.util.internal.Hidden.NettyBlockHoundIntegration for
SingleThreadEventExecutor.addTask.
* Also added unit tests for both SingleThreadEventExecutor.addTask
and GlobalEventExecutor.addTask.
Result:
SingleThreadEventExecutor.addTask can now be invoked from any thread
when BlockHound is activated.
Motivation:
When a HashedWheelTimer instance is started or stopped, its working
thread is started or stopped. These operations block the calling
thread:
- start() calls java.util.concurrent.CountDownLatch.await() to wait
for the worker thread to finish initializing;
- stop() calls java.lang.Thread.join(long) to wait for the worker
thread to exit.
BlockHound detects these calls and as a consequence, prevents
HashedWheelTimer from working properly, if it is started or stopped
in a thread that is not allowed to block.
Modifications:
Added two more BlockHound exceptions to
io.netty.util.internal.Hidden.NettyBlockHoundIntegration: one
for HashedWheelTimer.start() and one for HashedWheelTimer.stop().
Result:
HashedWheelTimer can now be started and stopped properly when
BlockHound is activated.
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:
There is no need for ByteProcessor to throw a checked exception.
The declared checked exception causes unnecessary code complications just to propagate it.
This can be cleaned up.
Modification:
ByteProcessor.process no longer declares to throw a checked exception, and all the places that were trying to cope with the checked exception have been simplified.
Result:
Simpler code.
Motivation:
There is no need for ByteProcessor to throw a checked exception.
The declared checked exception causes unnecessary code complications just to propagate it.
This can be cleaned up.
Modification:
ByteProcessor.process no longer declares to throw a checked exception, and all the places that were trying to cope with the checked exception have been simplified.
Result:
Simpler code.
Motivation:
Netty 5 will require Java 11 at a minimum, so this pre-Java 9 specific code will never be used.
Modification:
Removed the dead code.
Result:
Less code to maintain.
Motivation:
03aafb9cff did ensure we fail while loading a natibe library which is not compatible. While this is great it is still sometimes hard for people to understand what NoSuchMethodError means in this context.
Modifications:
If possible rethrow the NoSuchMethodError and provide some more hints about multiple versions of the shared library
Result:
Easier to understand for people why loading fails
Motivation:
ddebc10 added a few adjustments that are needed by io_uring that we will add as an incubator repository. Unfortunally we missed the changed in PlatformDependent*.
Modifications:
Add missing methods
Result:
Be able to compile io_uring code against core netty
Motivation:
Since GraalVM version 19.3.0, instances of java.net.InetAddress (and its subclasses Inet4Address and Inet6Address) are not allowed in native image heap (that is, they cannot be stored in static fields of classes initialized at build time or be reachable through static fields of such classes). When building a native image, it makes sense to initialize at build time as many classes as possible.
But some fields of some classes in Netty (for example, NetUtil.LOCALHOST4) contain InetAddress instances. If a program is using code path that makes it possible to reach such fields at build time initialization, it becomes impossible to build a native image initializing core Netty classes initialized at runtime. An example of such a program is a client that uses netty-dns.
Modifications:
- Add netty-testsuite-native-image-client Maven module to test that such an example program can be built after the corresponding fixes
- Add native-image.properties to resolver-dns module to move initialization of some classes to runtime (some of them are parsing configuration during initialization, so it makes no sense to initialize them at build time; for others, it's needed to avoid InetAddress reachability at build time)
- Add substitutions for NetUtil.LOCALHOST4, NetUtil.LOCALHOST6 and NetUtil.LOCALHOST to overcome the InetAddress-related prohibition
- Extract some initialization code from NetUtil to NetUtilInitializations to allow it to be used by the substitutions
Result:
A client program using netty-dns with --initialize-at-build-time=io.netty builds successfully
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:
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:
We had two unit tests that sometimes failed due timeouts. After insepecting these I noticed these can be improved to run faster while still do the right validation
Modifications:
- Only submit one task for execution per execute
- Cleanup
Result:
No test failures due timeout
