Commit Graph

7105 Commits

Author SHA1 Message Date
Norman Maurer
c3a73b7aaa Fix compile error introduced by 9f76d37b76 2017-01-11 12:30:31 +01:00
Norman Maurer
1e3336514e 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:13:30 +01:00
Norman Maurer
f77c6f713f Add unit test that shows LineBasedFrameDelimiter correctly splits line.
Motivation:

Thought there may be a bug so added a testcase to verify everything works as expected.

Modifications:

Added testcase

Result:

More test-coverage.
2017-01-11 08:00:59 +01:00
Scott Mitchell
9f76d37b76 ByteBufUtil.compare int underflow
Motivation:
ByteBufUtil.compare uses long arithmetic but doesn't check for underflow on when converting from long to int to satisfy the Comparable interface. This will result in incorrect comparisons and violate the Comparable interface contract.

Modifications:
- ByteBufUtil.compare should protect against int underflow

Result:
Fixes https://github.com/netty/netty/issues/6169
2017-01-10 11:44:16 -08:00
Norman Maurer
1ed7a27ba3 Use href in WebSocketServer javadoc
Motivation:

We should use <a href=....> in the javadoc of WebSocketServer. See [#6172].

Modifications:

Use href.

Result:

Fix link in xref.
2017-01-10 13:34:58 +01:00
Norman Maurer
55c39c44b0 Fix missleading comment in AbstractChannelHandlerContext.invokeHandler()
Motivation:

The comment on AbstractChannelHandlerContext.invokeHandler() is incorrect and missleading. See [#6177]

Modifications:

Change true to false to correct the comment.

Result:

Fix missleading and incorrect comment.
2017-01-10 12:05:47 +01:00
Shixiong Zhu
735b83bd22 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:54:02 +01:00
Max Zhuravkov
a6637be597 InternalThreadLocalMap.arrayList should create a reusable ArrayList only if arrayList field is NULL.
Motivation:

InternalThreadLocalMap.arrayList returns a new ArrayList every time it's called that defeats the purpose of having a reusable ArrayList.

Modification:

Modified InternalThreadLocalMap.arrayList to create an ArrayList only if arrayList field is NULL.

Result:

InternalThreadLocalMap.arrayList now creates a reusable ArrayList only if arrayList field is NULL.
2017-01-03 12:55:10 -08:00
Roger Kapsi
bc26e73cfe Detecting actual Channel write idleness vs. slowness
Motivation

The IdleStateHandler tracks write() idleness on message granularity but does not take into consideration that the client may be just slow and has managed to consume a subset of the message's bytes in the configured period of time.

Modifications

Adding an optional configuration parameter to IdleStateHandler which tells it to observe ChannelOutboundBuffer's state.

Result

Fixes https://github.com/netty/netty/issues/6150
2016-12-30 17:22:32 -08:00
Frederic BREGIER
bc4fbd6f39 Extends HttpPostRequestEncoder to support all methods except TRACE
Motivation:

In Netty, currently, the HttpPostRequestEncoder only supports POST, PUT, PATCH and OPTIONS, while the RFC 7231 allows with a warning that GET, HEAD, DELETE and CONNECT use a body too (but not TRACE where it is explicitely not allowed).
The RFC in chapter 4.3 says:
"A payload within a XXX request message has no defined semantics;
sending a payload body on a XXX request might cause some existing
implementations to reject the request."
where XXX can be replaced by one of GET, HEAD, DELETE or CONNECT.

Current usages, on particular in REST mode, tend to use those extra HttpMethods for such queries.

So this PR proposes to remove the current restrictions, leaving only TRACE as explicitely not supported.

Modification:

In the constructor, where the test is done, replacing all by checking only against TRACE, and adding one test to check that all methods are supported or not.

Result:

Fixes #6138.
2016-12-30 12:03:36 -08:00
Norman Maurer
fc45300ab3 Fix compile errors introduced by 0f069ae311 2016-12-21 22:06:36 +01:00
Norman Maurer
0f069ae311 Fix handling of FullHttpResponse when respond to HEAD in HttpServerCodec
Motivation:

cb139043f3 introduced special handling of response to HEAD requests. Due a bug we failed to handle FullHttpResponse correctly.

Modifications:

Correctly handle FullHttpResponse for HEAD requests.

Result:

Works as expected.
2016-12-21 20:53:36 +01:00
Jon Chambers
e6842b4561 Expose channel pool configuration to subclasses.
Motivation:

`SimpleChannelPool` subclasses are likely to override the `connectChannel` method, and are likely to clobber the cloned `Bootstrap` handler in the process. To allow subclasses to properly notify the pool listener of new connections, we should expose (at least) the `handler` property of the pool to subclasses.

Modifications:

Expose `SimpleChannelPool` properties to subclasses via `protected` getters.

Result:

Subclasses can now use the bootstrap, handler, health checker, and health-check-on-release preoperties from their superclass.
2016-12-21 20:51:56 +01:00
Norman Maurer
4298fc5524 Revert "Disallow setting logger factory twice"
This reverts commit 0dacd52007 which needs more thoughts and so will go into the next release.
2016-12-21 15:16:10 +01:00
Norman Maurer
c7dbb7b6b2 Ensure we use a MPMC queue in ThreadDeathWatcher as it may be used from multiple threads at the same time.
Motivation:

We used a MPSC queue in ThreadDeathWatcher and checked if it empty via isEmpty() from multiple threads if very unlucky. Depending on the implementation this is not safe and may even produce things like live-locks.

Modifications:

Change to use a MPMC queue.

Result:

No more risk to run into issues when multiple threads call watch(...) / unwatch(...) concurrently.
2016-12-21 07:31:45 +01:00
Scott Mitchell
23e8ff76c7 Add unit test for HttpObjectDecoder with message split on buffer boundaries
Motivation:
We should have a unit test which explicitly tests a HTTP message being split between multiple ByteBuf objects.

Modifications:
- Add a unit test to HttpRequestDecoderTest which splits a request between 2 ByteBuf objects

Result:
More unit test coverage for HttpObjectDecoder.
2016-12-20 13:07:21 -08:00
Malik Baktiyarov
b91426ff33 Added checkStartsWith option for WebSocketServerProtocolHandler
Motivation:

Enables optional .startsWith() matching of req.uri() with websocketPath.

Modifications:

New checkStartsWith boolean option with default false value added to both WebSocketServerProtocolHandler and WebSocketServerProtocolHandshakeHandler. req.uri() matching is based on this option.

Result:

By default old behavior matching via .equal() is preserved. To use checkStartsWith use constructor shortcut: new WebSocketServerProtocolHandler(websocketPath, true) or fill this flag on full form of constructor among other options.
2016-12-20 11:07:17 +01:00
Norman Maurer
6a0f30d0d1 Allow to disable deleting of the native library file after it is loaded.
Motivation:

When profiling it is sometimes needed to still have the native library file avaible. We should allow to disable the explicit deletion and just delete it when the JVM stops.

This is related to #6110

Modifications:

Add io.netty.native.deleteLibAfterLoading system property which allows to disable the explicit delete after laoding

Result:

Possible to profile native libraries better.
2016-12-16 08:28:34 +00:00
Norman Maurer
49c55cc509 Explicit disable support of SSL / TLS Compression
Motivation:

Our ReferenceCountedOpenSslEngine does not support compression so we should explicit disable it.
This is related to #3722.

Modifications:

Set SSL_OP_NO_COMPRESSION option.

Result:

Not use compression.
2016-12-16 08:26:06 +00:00
Norman Maurer
9b9666a3fe Remove usage of own Atomic*FieldUpdater in favor of JDKs
Motivation:

In later Java8 versions our Atomic*FieldUpdater are slower then the JDK implementations so we should not use ours anymore. Even worse the JDK implementations provide for example an optimized version of addAndGet(...) using intrinsics which makes it a lot faster for this use-case.

Modifications:

- Remove methods that return our own Atomic*FieldUpdaters.
- Use the JDK implementations everywhere.

Result:

Faster code.
2016-12-15 08:21:52 +00:00
Norman Maurer
140f36c67f Fail build when unused c code is detected
Motivation:

To keep our code clean we should fail the build when unused c code is detected.

Modifications:

- Add '-Wunused-variable' to build flags

Result:

Cleaner code.
2016-12-15 08:04:44 +00:00
Norman Maurer
ecdb2feff4 [#5831] HttpServerCodec cannot encode a respons e to HEAD
request with a 'content-encoding: chunked' header

Motivation:

It is valid to send a response to a HEAD request that contains a transfer-encoding: chunked header, but it is not valid to include a body, and there is no way to do this using the netty4 HttpServerCodec.

The root cause is that the netty4 HttpObjectEncoder will transition to the state ST_CONTENT_CHUNK and the only way to transition back to ST_INIT is through the encodeChunkedContent method which will write the terminating length (0\r\n\r\n\r\n), a protocol error when responding to a HEAD request

Modifications:

- Keep track of the method of the request and depending on it handle the response differently when encoding it.
- Added a unit test.

Result:

Correclty handle HEAD responses that are chunked.
2016-12-15 08:01:45 +00:00
Norman Maurer
9d759d2530 Ensure leak aware buffers correctly close the ResourceLeakTracker
Motivation:

We should assert that the leak aware buffers correctly close the ResourceLeakTracker in the unit tests.

Modifications:

- Keep track of NoopResourceLeakTrackers and check if these were closed once the test completes
- Fix bugs in tests so the buffers are all released.

Result:

Better tests for leak aware buffers
2016-12-08 19:49:03 +01:00
Norman Maurer
39c0befe99 [#6114] Increase test timeout for test introduced in c2f4daa739
Motivation:

c2f4daa739 added a unit test but used a too small test timeout.

Modifications:

Increase timeout.

Result:

Test should have enough time to complete on the CI.
2016-12-08 13:17:58 +01:00
Norman Maurer
10fcd65b5c Allow to run tests with extra commandline arguments
Motivation:

Often its useful to run the tests with different commandline arguments (like different system properties).

Modifications:

Introduce argLine.javaProperties which can be set from the commandline as well to add arguments that should be append when run the unit tests.

Result:

More flexible way to run the tests.
2016-12-08 10:49:20 +01:00
Jason Tedor
0dacd52007 Disallow setting logger factory twice
Motivation:

InternalLoggerFactory either sets a default logger factory
implementation based on the logging implementations on the classpath, or
applications can set a logger factory explicitly. If applications wait
too long to set the logger factory, Netty will have already set a logger
factory leading to some objects using one logging implementation and
other objets using another logging implementation. This can happen too
if the application tries to set the logger factory twice, which is
likely a bug in the application. Yet, the Javadocs for
InternalLoggerFactory warn against this saying that
InternalLoggerFactory#setLoggerFactory "should be called as early as
possible and shouldn't be called more than once". Instead, Netty should
guard against this.

Modications:

We replace the logger factory field with an atomic reference on which we
can do CAS operations to safely guard against it being set twice. We
also add an internal holder class that captures the static interface of
InternalLoggerFactory that can aid in testing.

Result:

The logging factory can not be set twice, and applications that want to
set the logging factory must do it before any Netty classes are
initialized (or the default logger factory will be set).
2016-12-08 10:48:20 +01:00
Norman Maurer
965cddcd5d Only schedule a ThreadDeathWatcher task if caches are used.
Motivation:

If caches are disabled it does not make sense to schedule a task that will free up memory consumed by the caches.

Modifications:

Do not schedule if caches are disabled.

Result:

Less overhead.
2016-12-08 10:36:36 +01:00
Norman Maurer
b1d6b303b8 Correctly handle the case when BUFFER_OVERFLOW happens during unwrap but the readable bytes are bigger then the expected applicationBufferSize
Motivation:

We need to ensure we handle the case when BUFFER_OVERFLOW happens during unwrap but the readable bytes are bigger then the expected applicationBufferSize. Otherwise we may produce an IllegalArgumentException as we will try to allocate a buffer with capacity < 0.

Modifications:

- Guard against this case.
- Ensure we not double release buffer on exception when doing unwrap.

Result:

No more exception when running testsuite with java 9.
2016-12-08 08:23:00 +01:00
Scott Mitchell
f342ab4234 Use GitHub Issue/PR Template Feature
Motivation:
GitHub recently added the ability to setup PR and Issue templates https://github.com/blog/2111-issue-and-pull-request-templates. We should take advantage of this feature to ensure Issues / PRs are properly formed.

Modifications:
- add a .github directory with a CONTRIBUTING.md, ISSUE_TEMPLATE.md, and PULL_REQUEST_TEMPLATE.md file

Result:
Fixes https://github.com/netty/netty/issues/6074.
2016-12-07 11:40:46 -08:00
Norman Maurer
ba24690e86 Ensure SSLErrorTest also works with boringssl
Motivation:

boringssl uses different messages for the ssl alerts which are all uppercase. As we try to match case as well this fails in SSLErrorTest as we expect lower-case.

This test was introduced by 9b7fb2f362.

Modifications:

Ensure we first translate everything to lower-case before doing the assert.

Result:

SSLErrorTest also pass when boringssl is used.
2016-12-07 16:09:20 +01:00
T Jake Luciani
06c785c9eb Include frame-pointer to improve epoll flamegraph tracing
Motivation:

When attempting to flamegraph netty w/ epoll it was noticed the stacks are lost going from
java to epoll lib.

Modifications:

added the -fno-omit-framepointer flag to compiler flags to ensure the fp is kept intact

Result:

Flamegraphs will now show native code in the same stack as java code using perf-java-flames
2016-12-07 08:06:41 +01:00
Scott Mitchell
443b0403df NIO ServerChannel shouldn't close because of Exception
Motivation:
e102a008b6 changed a conditional where previously the NIO ServerChannel would not be closed in the event of an exception.

Modifications:
- Restore the logic prior to e102a008b6 which does not automatically close ServerChannels for IOExceptions

Result:
NIO ServerChannel doesn't close automatically for an IOException.
2016-12-05 20:51:25 -08:00
Norman Maurer
bcfd3a493c Cleanup after commit fc1cdc991e 2016-12-05 12:18:52 +01:00
Norman Maurer
8eb158d83c Fix false-positives when using ResourceLeakDetector.
Motivation:

We need to ensure the tracked object can not be GC'ed before ResourceLeak.close() is called as otherwise we may get false-positives reported by the ResourceLeakDetector. This can happen as the JIT / GC may be able to figure out that we do not need the tracked object anymore and so already enqueue it for collection before we actually get a chance to close the enclosing ResourceLeak.

Modifications:

- Add ResourceLeakTracker and deprecate the old ResourceLeak
- Fix some javadocs to correctly release buffers.
- Add a unit test for ResourceLeakDetector that shows that ResourceLeakTracker has not the problems.

Result:

No more false-positives reported by ResourceLeakDetector when ResourceLeakDetector.track(...) is used.
2016-12-05 07:42:33 +01:00
Norman Maurer
afffe3a681 [#6095] Remove catching of ConcurrentModificationException as this can not happen.
Motivation:

We should not catch ConcurrentModificationException as this can never happen because things are executed on the EventLoop thread.

Modifications:

Remove try / catch

Result:

Cleaner code.
2016-12-04 18:59:30 +01:00
Norman Maurer
78fb4ecc53 Correctly not try to call handshake() when engine is already closed.
Motivation:

We need to ensure we not call handshake() when the engine is already closed. Beside this our implementation of isOutboundDone() was not correct as it not took the pending data in the outbound buffer into acount (which may be also generated as part of an ssl alert). Beside this we also called SSL_shutdown(...) while we were still in init state which will produce an error and so noise in the log with openssl later versions.

This is also in some extend related to #5931 .

Modifications:

- Ensure we not call handshake() when already closed
- Correctly implement isOutboundDone()
- Not call SSL_shutdown(...) when still in init state
- Added test-cases

Result:

More correct behaviour of our openssl SSLEngine implementation.
2016-12-04 08:59:11 +01:00
Norman Maurer
4d327463c7 Correct guard against non SSL data in ReferenceCountedOpenSslEngine
Motivation:

When non SSL data is passed into SSLEngine.unwrap(...) we need to throw an SSLException. This was not done at the moment. Even worse we threw an IllegalArgumentException as we tried to allocate a direct buffer with capacity of -1.

Modifications:

- Guard against non SSL data and added an unit test.
- Make code more consistent

Result:

Correct behaving SSLEngine implementation.
2016-12-04 08:49:19 +01:00
Norman Maurer
aa89f37c2a Support compiling netty with Java9
Motivation:

Java9 will be released soon so we should ensure we can compile netty with Java9 and run all our tests. This will help to make sure Netty will be usable with Java9.

Modification:

- Add some workarounds to be able to compile with Java9, note that the full profile is not supported with Java9 atm.
- Remove some usage of internal APIs to be able to compile on java9
- Not support Alpn / Npn and so not run the tests when using Java9 for now. We will do a follow up PR to add support.

Result:

Its possible to build netty and run its testsuite with Java9.
2016-12-03 20:41:19 +01:00
Norman Maurer
b5e40b2dde PooledByteBufAllocatorTest may has memory visiblity issues as it uses non concurrent queue
Motivation:

PooledByteBufAllocatorTest uses an ArrayQueue but access it from multiple threads (not concurrently but still from different threads). This may leak to memory visibility issues.

Modifications:

- Use a concurrent queue
- Some cleanup

Result:

Non racy test code.
2016-12-02 07:42:35 +01:00
Norman Maurer
c131ef9f6f Allow to disable caching in PooledByteBufAllocator for non FastThreadLocalThreads
Motivation:

If a user allocates a lot from outside the EventLoop we may end up creating a lot of caches in the PooledByteBufAllocator. This may be wasteful and so it may be useful for an other to configure that caches should only be used from within EventLoops.

Modifications:

Add new constructor which allows to configure the caching behaviour.

Result:

More flexible configuration of PooledByteBufAllocator possible
2016-12-02 07:40:52 +01:00
Norman Maurer
cfd016cc63 Small performance improvements in ResourceLeakDetector
Motivation:

42fba015ce changed the implemention of ResourceLeakDetector to improve performance. While this was done a branch was missed that can be removed. Beside this  using a Boolean as value for the ConcurrentMap is sub-optimal as when calling remove(key, value) an uncessary instanceof check and cast is needed on each removal.

Modifications:

- Remove branch which is not needed anymore
- Replace usage of Boolean as value type of the ConcurrentMap and use our own special type which only compute hash-code one time and use a == operation for equals(...) to reduce overhead present when using Boolean.

Result:

Faster and cleaner ResourceLeakDetector.
2016-12-01 21:33:50 +01:00
Norman Maurer
38b26a6145 Correctly mark EpollServerDomainSocketChannel.isActive() as true after bind is complete.]
Motivation:

We missed to set active = true in EpollServerDomainSocketChannel.doBind(...) which also means that channelActive(...) was never triggered.

Modifications:

Correct set active = true in doBind(...)

Result:

EpollServerDomainSocketChannel is correctly set to active when bound.
2016-12-01 21:26:51 +01:00
Norman Maurer
1b0a96620f Allow to build netty when sun.misc.Unsafe is not avaible or -Dio.netty.noUnsafe=true is used.
Motivation:

We support using Netty without sun.misc.Unsafe, so we should also support building it without it. This way we can also run all tests without sun.misc.Unsafe and so see if it works as expected.

Modifications:

Correctly skip tests that depend on sun.misc.Unsafe if its not present or -Dio.netty.noUnsafe=true is used.

Result:

Be able to build netty without sun.misc.Unsafe
2016-12-01 21:25:06 +01:00
Norman Maurer
5acf8c9b31 SwappedByteBuf.unwrap() should return wrapped buffer.
Motivation:

SwappedByteBuf.unwrap() not returned the wrapped buffer but the buffer that was wrapped by the original buffer. This is not correct.

Modifications:

Correctly return wrapped buffer and fix test.

Result:

SwappedByteBuf.unwrap() works as expected.
2016-12-01 21:22:46 +01:00
Norman Maurer
5b457c8a22 Ensure we not complete the same promise that may be failed because of outbound handler exception.
Motivation:

It's important that we do not pass in the original ChannelPromise to safeClose(...) as when flush(...) will throw an Exception it will be propagated to the AbstractChannelHandlerContext which will try to fail the promise because of this. This will then fail as it was already completed by safeClose(...).

Modifications:

Create a new ChannelPromise and pass it to safeClose(...).

Result:

No more confusing logs because of failing to fail the promise.
2016-12-01 21:20:23 +01:00
Norman Maurer
129fa9aef7 [#6015] Fix racy PooledByteBufAllocatorTests
Motivation:

We had a few tests PooledByteBufAllocatorTests which used parkNanos(...) to give a resource enough time to get destroyed. This is race and may not be good enough.

Modifications:

Ensure the ThreadCache is really destroyed.

Result:

No more racy tests that depend on ThreadCaches.
2016-12-01 10:18:15 +01:00
Scott Mitchell
24d0f3d45d SslHandlerTest ByteBuf leak
Motivation:
We are now more careful to flush alerts that are generated when errors occur. We should also be more careful in unit tests to release any buffers that may be queued due to potential errors wich result in alerts.

Modifications:
- When SslHandlerTest uses EmbeddedChannel we should always call finishAndReleaseAll

Results:
Fixes https://github.com/netty/netty/issues/6057
2016-12-01 06:51:14 +01:00
Jason Tedor
ba0542e397 Do not try to be unsafe when told not to be unsafe
Motivation:

Netty has a flag (io.netty.noUnsafe) for specifying to Netty to not be
unsafe. Yet, when initializing PlatformDependent0, Netty still tries to
be unsafe. For application that specify to Netty to not be unsafe and
run under a security manager, this can lead to an obnoxious (debug
level) stack trace. Since Netty was told not to be unsafe, Netty should
not try to be unsafe.

Modifications:

The initialization logic in PlatformDependent0 should take into account
that Netty was told not to be unsafe. This means that we need to
initialize PlatformDependent#IS_EXPLICIT_NO_UNSAFE as soon as possible,
before the static initializer for PlatformDependent0 has a chance to
run. Thus the following modifications are made:
 - initialize PlatformDependent#IS_EXPLICIT_NO_UNSAFE before any other
   code in PlatformDependent causes PlatformDependent0 to initialize
 - expose the value of PlatformDependent#IS_EXPLICIT_NO_UNSAFE for
   reading in PlatformDependent0
 - take the value of PlatformDependent#IS_EXPLICIT_NO_UNSAFE into
   account in PlatformDependent0

Result:

Netty does not try to be unsafe when told not to be unsafe.
2016-11-29 08:24:20 +01:00
Jason Tedor
f20e95b41b Do not eagerly initialize the logger factory
Motivation:

For applications that set their own logger factory, they want that
logger factory to be the one logger factory. Yet, Netty eagerly
initializes this and then triggers initialization of other classes
before the application has had a chance to set its preferred logger
factory.

Modifications:

With this commit there are two key changes:
 - Netty does not attempt to eagerly initialize the default logger
   factory, only doing so if the application layer above Netty has not
   already set a logger factory
 - do not eagerly initialize unrelated classes from the logger factory;
   while the motivation behind this was to initialize ThreadLocalRandom
   as soon as possible in case it has to block reading from /dev/random,
   this can be worked around for applications where it is problematic by
   setting securerandom.source=file:/dev/urandom in their Java system
   security policy (no, it is not less secure; do not even get me
   started on myths about /dev/random)

Result:

Netty uses the logger factory that the application prefers, and does not
initialize unrelated classes.
2016-11-29 08:18:17 +01:00
Derbylock
1b528de190 Removed final keyword from FixedChannelPool 2016-11-23 13:38:04 +01:00