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
Motivation:
We have a few classes in which we store and reuse static instances of various exceptions. When doing so it is important to also override fillInStacktrace() and so prevent the leak of the ClassLoader in the internal backtrace field.
Modifications:
- Add overrides of fillInStracktrace when needed
- Move ThrowableUtil usage in the static methods
Result:
Fixes https://github.com/netty/netty/pull/10686
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:
We check lots of numbers if it lies in a range. So it's better to add a method in `ObjectUtil` to check if a number lies inside a range.
Modification:
Added Range check method.
Result:
A faster and better way to check if a number lies inside a range.
Motivation:
DefaultAttributeMap::attr has a blocking behaviour on lookup of an existing attribute:
it can be made non-blocking.
Modification:
Replace the existing fixed bucket table using a locked intrusive linked list
with an hand-rolled copy-on-write ordered single array
Result:
Non blocking behaviour for the lookup happy path
Motivation:
We wish to use Unsafe as little as possible, and Java 8 allows us
to take some short-cuts or play some tricks with generics,
for the purpose of working around having to declare all checked
exceptions. Ideally all checked exceptions would be declared, but
the code base is not ready for that yet.
Modification:
The call to UNSAFE.throwException has been removed, so when we need
that feature, we instead use the generic exception trick.
In may cases, Java 8 allows us to throw Throwable directly. This
happens in cases where no exception is declared to be thrown in a
scope.
Finally, some warnings have also been fixed, and some imports have
been reorganised and cleaned up while I was modifying the files
anyway.
Result:
We no longer use Unsafe for throwing any exceptions.
Motivation:
writeUtf8 can suffer from inlining issues and/or megamorphic call-sites on the hot path due to ByteBuf hierarchy
Modifications:
Duplicate and specialize the code paths to reduce the need of polymorphic calls
Result:
Performance are more stable in user code
Motivation:
We're converting `Inet4Address` to `Integer` quite frequently so it's a good idea to keep that code in `NetUtils`.
Modification:
Added ipv4AddressToInt(Inet4Address) in NetUtils
Result:
Easy conversion of `Inet4Address` to `Integer`.
Motivation:
It makes no sense to remove the task when the underlying executor fails as we may be able to pick it up later. Beside this the used Queue doesnt support remove(...) and so will throw.
Modifications:
Remove the queue.remove(...) call
Result:
Fixes https://github.com/netty/netty/issues/10501.
Motivation:
We should pluck executors in round-robin, but at the 32-bit overflow boundary, the round-robin sequence was disrupted when the number of executors are not a power of 2.
Modifications:
Changed the index counter from a 32-bit to a 64-bit long. The overflow bug is still technically there, but it now takes so long to reach that it will never happen in practice. For example, 2^63 nanoseconds is almost 300 years.
Result:
The round-robin behaviour is now preserved in practice. This is a backport of https://github.com/netty/netty/pull/10468
Motivation:
GlobalEventExecutor#addTask may be called during SingleThreadEventExecutor shutdown.
May result in a blocking call, because GlobalEventExecutor#taskQueue is a BlockingQueue.
Modifications:
Add allowBlockingCallsInside configuration for GlobalEventExecutor#addTask.
Result:
Fixes#10257.
When BlockHound is installed, GlobalEventExecutor#addTask is not reported as a blocking call.
Motivation:
GlobalEventExecutor/SingleThreadEventExecutor#taskQueue is BlockingQueue.
Modifications:
Add allowBlockingCallsInside configuration for GlobalEventExecutor/SingleThreadEventExecutor#takeTask.
Result:
Fixes#9984
When BlockHound is installed, GlobalEventExecutor/SingleThreadEventExecutor#takeTask is not reported as a blocking call.
Motivation:
Recent Intellij versions are starting to anticipate
future versions of Java that include a
`java.lang.Record` class, and the Intellij compiler
gets confused by the `Record` class in our
`ResorceLeakDetector`.
Modification:
Rename our `Record` class to `TracerRecord`.
This matches what the class is doing, while avoiding
any future name clashes.
Result:
Intellij can now compile the project again, even when
configured to use a future (snapshot or early access)
version of Java.
Motivation:
SSLEngineImpl.unwrap(...) may call FileInputStream.read(...) internally when TLS1.3 is used. This will cause an BlockingOperationError when BlockHound is enabled.
For more details see https://mail.openjdk.java.net/pipermail/security-dev/2020-August/022271.html
Modifications:
- Add whitelist entry to BlockHound config for now
- Update NettyBlockHoundIntegrationTest to include testing for this workaround
Result:
No BlockingOperationError when TLS1.3 is used with JDK SSL implementation and BlockHound is enabled
Motivation:
GraalVM's native images built with native-image tool do not support Unsafe.staticFieldOffset() method (at least, currently). If an application using Netty (and causing initialization of io.netty.util.internal.PlatformDependent0 class) is built into a native image and run, this results in the following error thrown during initialization:
Exception in thread "main" com.oracle.svm.core.jdk.UnsupportedFeatureError: Unsupported method of Unsafe
at com.oracle.svm.core.util.VMError.unsupportedFeature(VMError.java:86)
at jdk.internal.misc.Unsafe.staticFieldOffset(Unsafe.java:230)
at sun.misc.Unsafe.staticFieldOffset(Unsafe.java:662)
at io.netty.util.internal.PlatformDependent0$5.run(PlatformDependent0.java:294)
at java.security.AccessController.doPrivileged(AccessController.java:83)
at io.netty.util.internal.PlatformDependent0.<clinit>(PlatformDependent0.java:279)
This seems to be the reason of the behavior described in #10051.
Modification:
The idea of this commit is to only invoke Unsafe.staticFieldOffset() is we are not in a native image; if we are, behave like if we could not find the field at all.
GraalDetector is borrowed from Spring framework.
Result:
Fixes#10051
Motivation:
We should provide details about why an IOOBE was thrown
Modification:
Add IndexOutOfBoundsException error information in io.netty.util.internal.AppendableCharSequence and io.netty.handler.codec.CodecOutputList class
Result:
Easier to debug
Motivation:
When a switch statement is used we should always define a `default:` so we don't introduce bugs due fall-through.
Modification:
Add missing `default:`s
Result:
Less error-prone code
Motivation:
When BlockHound is installed,
ReferenceCountedOpenSslClientContext$ExtendedTrustManagerVerifyCallback.verify
is reported as blocking call.
Modifications:
Add allowBlockingCallsInside configuration for
ReferenceCountedOpenSslClientContext$ExtendedTrustManagerVerifyCallback.verify
Result:
Fixes#10384
Motivation:
newResourceLeakDetector(...) did not correctly pass the samplingInterval parameter and so it was ignored.
Modification:
ResourceLeakDetectorFactory.newResourceLeakDetector(Class, int) use the second parameter as the sampling interval of the newly created ResourceLeakDetector.
Result:
Fixes#10378
Motivation:
Unsafe users are getting MpscChunkedArrayQueue while no Unsafe ones MpscGrowableAtomicArrayQueue
Modifications:
MpscChunkedAtomicArrayQueue should be used for no Unsafe users (matching MpscChunkedArrayQueue behaviour)
Result:
no Unsafe users uses MpscChunkedAtomicArrayQueue while allocating bounded Mpsc Queues
Motivation:
Fix very tiny comment error in Recycler
Modifications:
Fix very tiny comment error in Recycler
Result:
Correctly comment about drop in WeakOrderQueue#add
Motivation
- Recycler stack and delayed queue drop ratio can only be configured with
the same value. The overall drop ratio is ratio^2.
- #10251 shows that enable drop in `WeakOrderQueue` may introduce
performance degradation. Though the final reason is not clear now,
it would be better to add option to configure delayed queue drop ratio separately.
Modification
- "io.netty.recycler.delayedQueue.ratio" as the drop ratio of delayed queue
- default "delayedQueue.ratio" is same as "ratio"
Results
Able to configure recycler delayed queue drop ratio separately
Motivation
1. It's inable to collect all object because RATIO is always >=1 after
`safeFindNextPositivePowerOfTwo`
2. Enable drop object in `WeakOrderQueue`(commit:
71860e5b94) enlarge the drop ratio. We
can subtly control the overall drop ratio by using `io.netty.recycler.ratio` directly,
Modification
- Remove `safeFindNextPositivePowerOfTwo` before set the ratio
Results
Able to disable drop when recycle object
Motivation:
We have found out that ByteBufUtil.indexOf can be inefficient for substring search on
ByteBuf, both in terms of algorithm complexity (worst case O(needle.readableBytes *
haystack.readableBytes)), and in constant factor (esp. on Composite buffers).
With implementation of more performant search algorithms we have seen improvements on
the order of magnitude.
Modifications:
This change introduces three search algorithms:
1. Knuth Morris Pratt - classical textbook algorithm, a good default choice.
2. Bit mask based algorithm - stable performance on any input, but limited to maximum
search substring (the needle) length of 64 bytes.
3. Aho–Corasick - worse performance and higher memory consumption than [1] and [2], but
it supports multiple substring (the needles) search simultaneously, by inspecting every
byte of the haystack only once.
Each algorithm processes every byte of underlying buffer only once, they are implemented
as ByteProcessor.
Result:
Efficient search algorithms with linear time complexity available in Netty (I will share
benchmark results in a comment on a PR).
Motivation:
How we did wildcard matching was not correct according to RFC6125. Beside this our implementation was quite CPU heavy.
Modifications:
- Add new DomainWildcardMappingBuilder which correctly does wildcard matching. See https://tools.ietf.org/search/rfc6125#section-6.4
- Add unit tests
- Deprecate old implementations
Result:
Correctly implement wildcard matching and improve performance
Motivation:
As we have java8 as a minimum target we can use MethodHandles. We should do so when we expect to have a method called multiple times.
Modifications:
- Replace usage of reflection with MethodHandles where it makes sense
- Remove some code which was there to support java < 8
Result:
Faster code
Motivation:
Magic numbers seem hard to read or understand.
Modification:
Replace several magic numbers with named constants.
Result:
Improve readability and make it easier to maintain.
Motivation:
ThrowableUtil.stackTraceToString is an expensive method call. So I think a log level check before this logging statement is quite needed especially in a environment with the warning log disabled.
Modification:
Add log level check simply before logging.
Result:
Improve performance in a environment with the warning log disabled.
Motivation:
In general, we will close the debug log in a product environment. However, logging without external level check may still affect performance as varargs will need to allocate an array.
Modification:
Add log level check simply before logging.
Result:
Improve performance slightly in a product environment.
Motivation:
ImmediateExecutor needs more test cases.
Modification:
Add several test cases for ImmediateExecutor.
Result:
Improve test coverage slightly.
Motivation:
Our parsing of the initial line / http headers did treat some characters as separators which should better trigger an exception during parsing.
Modifications:
- Tighten up parsing of the inital line by follow recommentation of RFC7230
- Restrict separators to OWS for http headers
- Add unit test
Result:
Stricter parsing of HTTP1
Motivation:
NetworkInterface.getByInetAddress() may return null on Android. This is incorrect by the API but still happens. To help our users we should provide a workaround
Modifications:
Just return an empty Enumeration when null is returned.
Result:
Fixes https://github.com/netty/netty/issues/10045
Motivation:
BufferedReader.readLine() may return null and cause a NPE.
Modification:
Simply add a null check.
Result:
If BufferedReader.readLine() returns null, the sysctlGetInt will just return null rather than cause NPE.
Motivation:
Modifications:
- Wrap the code and execute with an AccessController
- Ignore SecurityException (by just logging it)
- Add some more debug logging
Result:
Fixes https://github.com/netty/netty/issues/10017
Motivation:
If there was always a task in the taskQueue of GlobalEvenExecutor, scheduled tasks in the
scheduledTaskQueue will never be executed.
Related to #1614
Modifications:
fix bug in GlobalEventExecutor#takeTask
Result:
fix bug
Motivation:
JDK is the default SSL provider and internally uses blocking IO operations.
Modifications:
Add allowBlockingCallsInside configuration for SslHandler runAllDelegate function.
Result:
When BlockHound is installed, SSL works out of the box with the default SSL provider.
Co-authored-by: violetagg <milesg78@gmail.com>
Motivation:
Deploying a Micronaut application as GraalVM native image to AWS Lambda with custom runtime fails when using Micronaut Http Client.
This PR initializes at runtime some classes needed to fix the issue. There is more information in our original issue in Micronaut https://github.com/micronaut-projects/micronaut-core/issues/2335#issuecomment-570151944
At this moment I've added those classes into Micronaut (b383d3ab14) as a workaround but this should be included in Netty so it's available for everyone.
Modification:
Mark 3 classes to be initialized at runtime for GraalVM.
Result:
Mark 3 classes to be initialized at runtime for GraalVM.
Motivation:
We can extend `ResourceLeakDetector` through `ResourceLeakDetectorFactory`, and then report the leaked information by covering `reportTracedLeak` and `reportUntracedLeak`. However, the behavior of `reportTracedLeak` and `reportUntracedLeak` is controlled by `logger.isErrorEnabled()`, which is not reasonable. In the case of extending `ResourceLeakDetector`, we sometimes need `needReport` to always return true instead of relying on `logger.isErrorEnabled ()`.
Modification:
introduce `needReport` method and let it be `protected`
Result:
We can control the report leak behavior.
Motivation:
decodeHexNibble can be a lot faster using a lookup table
Modifications:
decodeHexNibble is made faster by using a lookup table
Result:
decodeHexNibble is faster
Motivation:
The resolv.conf file may contain inline comments which should be ignored
Modifications:
- Detect if we have a comment after the ipaddress and if so skip it
- Add unit test
Result:
Fixes https://github.com/netty/netty/issues/9889
