Commit Graph

853 Commits

Author SHA1 Message Date
Norman Maurer
9e6e1a3e7b Use SystemPropertyUtil to access system properties
Motivation:

We should use SystemPropertyUtil to access system properties and so always handle SecurityExceptions.

Modifications:

Use SystemPropertyUtil everywhere.

Result:

Better and consist handling of SecurityException.
2017-03-19 08:09:29 -07:00
Norman Maurer
2b8c8e0805 [maven-release-plugin] prepare for next development iteration 2017-03-10 07:46:17 +01:00
Norman Maurer
1db58ea980 [maven-release-plugin] prepare release netty-4.1.9.Final 2017-03-10 07:45:28 +01:00
Norman Maurer
e12f504ac1 Remove deprecated usage of Mockito methods
Motivation:

We used some deprecated Mockito methods.

Modifications:

- Replace deprecated method usage
- Some cleanup

Result:

No more usage of deprecated Mockito methods. Fixes [#6482].
2017-03-09 20:59:54 +01:00
Norman Maurer
9ade81ab5b Use system property to detect if root is running the program
Motivation:

We forked a new process to detect if the program is run by root. We should better just use user.name system property

Modifications:

- Change PlatformDependent.isRoot0() to read the user.name system property to detect if root runs the program and rename it to maybeSuperUser0().
- Rename PlatformDependent.isRoot() to maybeSuperUser() and let it init directly in the static block

Result:

Less heavy way to detect if the program is run by root.
2017-03-09 11:16:10 +01:00
Norman Maurer
c6a3cae269 UnorderedThreadPoolEventExecutor consumes 100% CPU when idle
Motivation:

When UnorderedThreadPoolEventExecutor.execute / submit etc is called it will consume up to 100 % CPU even after the task was executed.

Modifications:

Add a special wrapper which we will be used in execute(...) to wrap the submitted Runnable. This is needed as  ScheduledThreadPoolExecutor.execute(...) will delegate to submit(...) which will then use decorateTask(...). The problem with this is that decorateTask(...) needs to ensure we only do our own decoration if we not call from execute(...) as otherwise we may end up creating an endless loop because DefaultPromise will call  EventExecutor.execute(...) when notify the listeners of the promise.

Result:

Fixes [#6507].
2017-03-09 11:12:42 +01:00
Nikolay Fedorovskikh
2993760e92 Fix misordered 'assertEquals' arguments in tests
Motivation:

Wrong argument order in some 'assertEquals' applying.

Modifications:

Flip compared arguments.

Result:

Correct `assertEquals` usage.
2017-03-08 22:48:37 -08:00
Scott Mitchell
8b21cd9e35 PlatformDependent0 should enforce array index scale for byte[] explicitly
Motivation:
PlatformDependent0 makes assumptions that the array index scale for byte[] is always 1. If this is not the case the results from methods which make this assumption will be undefined.

Modifications:
- PlatformDependent0 should check if unsafe.arrayIndexScale(byte[].class) is not 1, and if so not use unsafe

Result:
Assumptions made by optimizations in PlatformDependent0 which use byte[] are explicitly enforced.
2017-03-08 10:02:37 -08:00
Norman Maurer
1e5d33f8d5 Remove unused code
Motivation:

Cleanup PlatformDependent* and remove unused code.

Modifications:

Code cleanup

Result:

Removed unused code
2017-03-07 21:33:41 +01:00
Norman Maurer
1392bc351f Correctly build socketaddress string, followup of 8b2badf44f 2017-03-01 20:05:20 +01:00
Norman Maurer
0514b0c61b Only add port to HOST header value if needed
Motivation:

We only need to add the port to the HOST header value if its not a standard port.

Modifications:

- Only add port if needed.
- Fix parsing of ipv6 address which is enclosed by [].

Result:

Fixes [#6426].
2017-03-01 19:08:19 +01:00
Nikolay Fedorovskikh
943f4ec7ff Make methods 'static' where it missed
Motivation:

Calling a static method is faster then dynamic

Modifications:

Add 'static' keyword for methods where it missed

Result:

A bit faster method calls
2017-02-23 11:01:57 +01:00
Norman Maurer
7d08b4fc35 Remove optional dependency on javassist
Motivation:

We shipped a javassist based implementation for typematching and logged a confusing debug message about missing javassist. We never were able to prove it really gives any perf improvements so we should just remove it.

Modifications:

- Remove javassist dependency and impl
- Fix possible classloader deadlock as reported by intellij

Result:

Less code to maintain and less confusing log message.
2017-02-23 07:54:42 +01:00
Nikolay Fedorovskikh
0623c6c533 Fix javadoc issues
Motivation:

Invalid javadoc in project

Modifications:

Fix it

Result:

More correct javadoc
2017-02-22 07:31:07 +01:00
Nikolay Fedorovskikh
634a8afa53 Fix some warnings at generics usage
Motivation:

Existing warnings from java compiler

Modifications:

Add/fix type parameters

Result:

Less warnings
2017-02-22 07:29:59 +01:00
Norman Maurer
67be7c5b9f Log why it was not possible to use ByteBuffer.cleaner
Motivation:

We should log why we can not use ByteBuffer.cleaner and so maybe allow the user to fix it.

Modifications:

- Use Unsafe to access the field
- Log the exception when we can not use ByteBuffer.cleaner

Result:

Easier to debug why using cleaner is not possible.
2017-02-17 07:34:34 +01:00
Norman Maurer
fbf0e5f4dd Prefer JDK ThreadLocalRandom implementation over ours.
Motivation:

We have our own ThreadLocalRandom implementation to support older JDKs . That said we should prefer the JDK provided when running on JDK >= 7

Modification:

Using ThreadLocalRandom implementation of the JDK when possible.

Result:

Make use of JDK implementations when possible.
2017-02-16 15:44:00 -08:00
Scott Mitchell
4d7d478a3d Update JCTools to 2.0.1 2017-02-16 15:09:35 -08:00
Norman Maurer
6ac5f35077 Use Unsafe to read ByteBuffer.address field to make it work on Java9 as well.
Motivation:

Java9 does not allow changing access level via reflection by default. This lead to the situation that netty disabled Unsafe completely as ByteBuffer.address could not be read.

Modification:

Use Unsafe to read the address field as this works on all Java versions.

Result:

Again be able to use Unsafe optimisations when using Netty with Java9
2017-02-16 20:40:59 +01:00
Norman Maurer
1843b31885 Guard against having NetworkInterface.getNetworkInterfaces() return null
Motivation:

NetworkInterface.getNetworkInterfaces() may return null if no network interfaces are found. We should guard against it.

Modifications:

Check for null return value.

Result:

Fixes [#6384]
2017-02-16 07:59:31 +01:00
Jason Tedor
d59b4840c1 Cleanup ChannelId handling of basic methods
Motiviation:

Simplify implementation of compareTo/equals/hashCode for ChannelIds.

Modifications:

We simplfy the hashCode implementation for DefaultChannelId by not
making it random, but making it based on the underlying data. We fix the
compareTo implementation for DefaultChannelId by using lexicographic
comparison of the underlying data array. We fix the compareTo
implementation for CustomChannelId to avoid the possibility of overflow.

Result:

Cleaner code that is easier to maintain.
2017-02-15 11:53:36 -08:00
Norman Maurer
84188395be Remove backports of JDK8 classes
Motivation:

Java8 is out now for some time and JDK7 is no longer supported officially. We should remove all our backports and just use what the JDK provides us. This also will allow us to use intrinsics that are offered by the JDK implementations.

Modifications:

Remove all backports of jdk8 classes.

Result:

Use what the JDK offers us. This also fixes [#5458]
2017-02-15 20:44:23 +01:00
Dmitriy Dumanskiy
506f0d8f8c Cleanup : String.length() == 0 replaced with String.isEmpty, removed unnecessary assert, class cast 2017-02-14 15:36:42 +01:00
Norman Maurer
90bc605477 Initialization of PlatformDependent0 fails on Java 9
Motivation:

Initialization of PlatformDependent0 fails on Java 9 in static initializer when calling setAccessible(true).

Modifications:

Add RefelectionUtil which can be used to safely try if setAccessible(true) can be used or not and if not fail back to non reflection.

Result:

Fixed [#6345]
2017-02-14 10:15:27 +01:00
fenik17
0cf3f54a8d Adding 'final' keyword for private fields where possible
Motivation

Missing 'final' keyword for fields

Modifications

Add 'final' for fields where possible

Result

More safe and consistent code
2017-02-14 08:29:15 +01:00
Scott Mitchell
a1b5b5dcca EpollRecvByteAllocatorHandle doesn't inform delegate of more data
Motivation:
EpollRecvByteAllocatorHandle intends to override the meaning of "maybe more data to read" which is a concept also used in all existing implementations of RecvByteBufAllocator$Handle but the interface doesn't support overriding. Because the interfaces lack the ability to propagate this computation EpollRecvByteAllocatorHandle attempts to implement a heuristic on top of the delegate which may lead to reading when we shouldn't or not reading data.

Modifications:
- Create a new interface ExtendedRecvByteBufAllocator and ExtendedHandle which allows the "maybe more data to read" between interfaces
- Deprecate RecvByteBufAllocator and change all existing implementations to extend ExtendedRecvByteBufAllocator
- transport-native-epoll should require ExtendedRecvByteBufAllocator so the "maybe more data to read" can be propagated to the ExtendedHandle

Result:
Fixes https://github.com/netty/netty/issues/6303.
2017-02-13 17:42:24 -08:00
Scott Mitchell
54c9ecf682 DnsNameResolver should respect /etc/resolv.conf and /etc/resolver
Motivation:
The JDK uses gethostbyname for blocking hostname resoltuion. gethostbyname can be configured on Unix systems according to [1][2]. This may impact the name server that is used to resolve particular domains or just override the default fall-back resolver. DnsNameResolver currently ignores these configuration files which means the default resolution behavior is different than the JDK. This may lead to unexpected resolution failures which succeed when using the JDK's resolver.

Modifications:
- Add an interface which can override what DnsServerAddressStream to use for a given hostname
- Provide a Unix specific implementation of this interface and implement [1][2]. Some elements may be ignored sortlist, timeout, etc...

Result:
DnsNameResolver behaves more like the JDK resolver by default.

[1] https://linux.die.net/man/5/resolver
[2] https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man5/resolver.5.html
2017-02-13 11:54:09 -08:00
md-5
fe522fb18e Update Groovy to 2.4.8 for Java 9-ea+154 support.
Motivation:

Update of Groovy is needed to compile on recent java9 releases.

Modification:

Update to Groovy 2.4.8

Result:

This change allows Netty to be successfully compiled on more recent Java 9 previews.
2017-02-11 14:09:15 +01:00
Dmitriy Dumanskiy
c95517f759 Cleanup : removed unnecessary 'continue', explicit array creation, unwrapping 2017-02-10 12:25:01 +01:00
Scott Mitchell
14b902fced Deprecate and ignore ResourceLeakDetector's maxActive parameter
Motivation:
ResourceLeakDetector supports a parameter called maxActive. This parameter is used in attempt to limit the amount of objects which are being tracked for leaks at any given time, and generates an error log message if this limit is exceeded. This assumes that there is a relationship between leak sample rate and object lifetime for objects which are already being tracked. This relationship may appear to work in cases were there are a single leak record per object and those leak records live for the lifetime of the application but in general this relationship doesn't exist. The original motivation was to provide a limit for cases such as HashedWheelTimer to limit the number of instances which exist at any given time. This limit is not enforced in all circumstances in HashedWheelTimer (e.g. if the thread is a daemon) and can be implemented outside ResourceLeakDetector.

Modifications:
- Deprecate all methods which interact with maxActive in ResourceLeakDetectorFactory and ResourceLeakDetector
- Remove all logic related to maxActive in ResourceLeakDetector
- HashedWheelTimer implements its own logic to impose a limit and warn users if too many instances exists at any given time.

Result:
Fixes https://github.com/netty/netty/issues/6225.
2017-02-08 19:59:58 -08:00
Norman Maurer
a7c0ff665c Only use Mockito for mocking.
Motivation:

We used various mocking frameworks. We should only use one...

Modifications:

Make usage of mocking framework consistent by only using Mockito.

Result:

Less dependencies and more consistent mocking usage.
2017-02-07 08:47:22 +01:00
Jason Tedor
42c0359820 Do not prefer empty MAC address
Motivation:

When comparing MAC addresses searching for the best MAC address, if
locally-administered address (e.g., from a Docker container) is compared
against an empty MAC address, the empty MAC address will be marked as
preferred. In cases this is the only available MAC address, this leaves
Netty using a random machine ID instead of using a perfectly valid
machine ID from the locally-adminstered address.

Modifications:

This commit modifies the MAC address logic so that the empty MAC address
is not preferred over a locally-administered address. This commit also
simplifies the comparison logic here.

Result:

Empty MAC addresses will not be preferred over locally-administered
addresses thus permitting the default machine ID to be the
locally-adminstered MAC address if it is the only available MAC address.
2017-02-06 12:14:27 -08:00
Scott Mitchell
3482651e0c HTTP/2 Non Active Stream RFC Corrections
Motivation:
codec-http2 couples the dependency tree state with the remainder of the stream state (Http2Stream). This makes implementing constraints where stream state and dependency tree state diverge in the RFC challenging. For example the RFC recommends retaining dependency tree state after a stream transitions to closed [1]. Dependency tree state can be exchanged on streams in IDLE. In practice clients may use stream IDs for the purpose of establishing QoS classes and therefore retaining this dependency tree state can be important to client perceived performance. It is difficult to limit the total amount of state we retain when stream state and dependency tree state is combined.

Modifications:
- Remove dependency tree, priority, and weight related items from public facing Http2Connection and Http2Stream APIs. This information is optional to track and depends on the flow controller implementation.
- Move all dependency tree, priority, and weight related code from DefaultHttp2Connection to WeightedFairQueueByteDistributor. This is currently the only place which cares about priority. We can pull out the dependency tree related code in the future if it is generally useful to expose for other implementations.
- DefaultHttp2Connection should explicitly limit the number of reserved streams now that IDLE streams are no longer created.

Result:
More compliant with the HTTP/2 RFC.
Fixes https://github.com/netty/netty/issues/6206.

[1] https://tools.ietf.org/html/rfc7540#section-5.3.4
2017-02-01 10:34:27 -08:00
Norman Maurer
735d6dd636 [maven-release-plugin] prepare for next development iteration 2017-01-30 15:14:02 +01:00
Norman Maurer
76e22e63f3 [maven-release-plugin] prepare release netty-4.1.8.Final 2017-01-30 15:12:36 +01:00
Jon Chambers
94cb389c04 Restore add(Promise) and addAll(Promise...) methods to PromiseCombiner.
Motivation:

A testing goof in 7c630fe introduced a binary incompatibility when the old Promise-specific `add` and `addAll` methods in PromiseCombiner were generalized to accept `Futures`.

Modification:

- Restore (but mark as `@Deprecated`) old PromiseCombiner methods.
- Fixed a couple minor documentation typos because sure why not.

Result:

`PromiseCombiner` is binary-compatible with previous versions of Netty.
2017-01-30 09:23:11 +01:00
Norman Maurer
a416b79d86 DnsNameResolver.resolve*(...) never notifies the Future when empty hostname is used.
Motivation:

When an empty hostname is used in DnsNameResolver.resolve*(...) it will never notify the future / promise. The root cause is that we not correctly guard against errors of IDN.toASCII(...) which will throw an IllegalArgumentException when it can not parse its input. That said we should also handle an empty hostname the same way as the JDK does and just use "localhost" when this happens.

Modifications:

- If the try to resolve an empty hostname we use localhost
- Correctly guard against errors raised by IDN.toASCII(...) so we will always noify the future / promise
- Add unit test.

Result:

DnsNameResolver.resolve*(...) will always notify the future.
2017-01-24 21:21:49 +01:00
Tim Brooks
3344cd21ac Wrap operations requiring SocketPermission with doPrivileged blocks
Motivation:

Currently Netty does not wrap socket connect, bind, or accept
operations in doPrivileged blocks. Nor does it wrap cases where a dns
lookup might happen.

This prevents an application utilizing the SecurityManager from
isolating SocketPermissions to Netty.

Modifications:

I have introduced a class (SocketUtils) that wraps operations
requiring SocketPermissions in doPrivileged blocks.

Result:

A user of Netty can grant SocketPermissions explicitly to the Netty
jar, without granting it to the rest of their application.
2017-01-19 21:12:52 +01:00
Norman Maurer
71efb0b39e Do not replace System.err during Slf4JLoggerFactory construction
Motivation:

Replacing System.err during Slf4JLoggerFactory construction is problematic as another class may optain the System.err reference before we set it back to the original value.

Modifications:

Remove code that temporary replaced System.err.

Result:

Fixes [#6212].
2017-01-19 19:28:01 +01:00
Dmitriy Dumanskiy
0e654f77e2 Level initialization cleanup. 2017-01-19 10:11:38 -08:00
Dmitriy Dumanskiy
141554f5d1 Removed unnecessary pattern matching during number paring and unnecessary toLowerCase() invocation.
Motivation:

Pattern matching not necessary for number parsing.

Modification:

Removed pattern matching for number parsing and removed unnecessary toLowerCase() operation.

Result:

No static variable with pattern, removed unnecessary matching operation and toLowerCase() operation.
2017-01-19 10:11:55 +01:00
Norman Maurer
4f6ef69582 Follow-up cleanup of 0c4826586f 2017-01-19 08:01:14 +01:00
Norman Maurer
0c4826586f Cleanup PlatformDependent* code
Motivation:

PlatformDependent* contains some methods that are not used and some other things that can be cleaned-up.

Modifications:

- Remove unused methods
- cleanup

Result:

Code cleanup.
2017-01-19 07:55:18 +01:00
Norman Maurer
7f01da8d0f [maven-release-plugin] prepare for next development iteration 2017-01-12 11:36:51 +01:00
Norman Maurer
7a21eb1178 [maven-release-plugin] prepare release netty-4.1.7.Final 2017-01-12 11:35:58 +01:00
Norman Maurer
eb5dc4bced Correctly handle IPV6 in HttpProxyHandler
Motivation:

The HttpProxyHandler is expected to be capable of issuing a valid CONNECT request for a tunneled connection to an IPv6 host.

Modifications:

- Correctly format the IPV6 address.
- Add unit tests

Result:

HttpProxyHandler works with IPV6 as well. Fixes [#6152].
2017-01-12 07:51:37 +01:00
Scott Mitchell
ec3d077e0d DefaultHttp2Connection modifying child map while iterating
Motivation:
When DefaultHttp2Connection removes a stream it iterates over all children and adds them as children to the parent of the stream being removed. This process may remove elements from the child map while iterating without using the iterator's remove() method. This is generally unsafe and may result in an undefined iteration.

Modifications:
- We should use the Iterator's remove() method while iterating over the child map

Result:
Fixes https://github.com/netty/netty/issues/6163
2017-01-11 11:07:18 -08:00
Norman Maurer
3c5e677964 Add assert to ensure we not create an endless loop and fix unsafe publication
Motivation:

[#6153] reports an endless loop that existed in the Recycler, while this was fixed adding a few asserts to ensure this remains fixed is a good thing. Beside this we also should ensure this can not escape the constructor to avoid unsafe publication.

Modifications:

- Add asserts
- Fix unsafe publication

Result:

More correct code.
2017-01-11 12:14:27 +01:00
Norman Maurer
2368f238ad Fix typo in inner-class name
Motivation:

There is a typo in the inner-class name.

Modifications:

Fix typo.

Result:

One typo less. Fixes [#6185].
2017-01-10 13:49:52 +01:00
Shixiong Zhu
2457f386d8 Set prev to null when setting cursor to head in scavengeSome.
Motivation:

`scavengeSome()` has a corner case: when setting `cursor` to `head`, `this.prev` may point to the tail of the  `WeakOrderQueue` linked list. Then it's possible that the following while loop will link the tail to the head, and cause endless loop.

I made a reproducer in 36522e7b72 . The unit test will just run forever. Unfortunately, I cannot change it to a unit test because it needs to add some codes to `scavengeSome` to control the execution flow.

Modification:

Set `prev` to null when setting `cursor` to `head` in `scavengeSome`

Result:

Fixes #6153.
2017-01-07 20:48:46 +01:00