Commit Graph

6972 Commits

Author SHA1 Message Date
nmittler
27b330d8b1 Moving KObjectHashMapTest to propert directory
Motivation:

The KObjectHashMapTest is in a directory called "io.netty.util.collection" rather than "io/netty/util/collection". This causes the generated tests to be created in the wrong directory as well.

Modifications:

Moved the file.

Result:

Fixes #4546
2015-12-14 10:08:41 -08:00
Stephane Landelle
6393506b97 Extract SocketAdress logic from NameResolver
Motivation:

As discussed in #4529, NameResolver design shouldn't be resolving SocketAddresses (or String name + port) and return InetSocketAddresses. It should resolve String names and return InetAddresses.
This SocketAddress to InetSocketAddresses resolution is actually a different concern, used by Bootstrap.

Modifications:

Extract SocketAddress to InetSocketAddresses resolution concern to a new class hierarchy named AddressResolver.
These AddressResolvers delegate to NameResolvers.

Result:

Better separation of concerns.

Note that new AddressResolvers generate a bit more allocations because of the intermediate Promise and List<InetAddress>.
2015-12-14 14:03:50 +01:00
Norman Maurer
89ff831a67 [#4449] Remove registered events from eventloop before close
Motivation:

We need to remove all registered events for a Channel from the EventLoop before doing the actual close to ensure we not produce a cpu spin when the actual close operation is delayed or executed outside of the EventLoop.

Modifications:

Deregister for events for NIO and EPOLL socket implementations when SO_LINGER is used.

Result:

No more cpu spin.
2015-12-13 09:55:50 +01:00
Brendt Lucas
e8eda1b99f Fix AsciiString.contentEqualsIgnoreCase
Motivation:

Related to issue #4564.

AsciiString.contentEqualsIgnoreCase fails when comparing two AsciiStrings of the same length

Modifications:

Compare the values of the first AsciiString to the second AsciiString

Result:

AsciiString.contentEqualsIgnoreCase works as expected
2015-12-12 19:48:17 +01:00
Fabian Lange
e5386b05e6 Move Hex dump related util from ByteBufUtil to inner class
Motivation:

Initialisation of the ByteBufUtil class, a class frequently used is
delayed because a significant number of String operations is performed to
fill a HEXDUMP_ROWPREFIXES array. This array also sticks to the Strings
forever.
It is quite likely that applications never use the hexdump facility.

Modification:

Moved the static initialisation and references to a static inner class.
This delays initialisation (and memory usage) until actually needed.
The API is kept as is.

Result:

Faster startup time, less memory usage for most netty using applications.
2015-12-11 19:47:57 +01:00
Scott Mitchell
1d0b6ad75e DefaultPromiseTest dead code removal
Motivation:
DefaultPromiseTest has dead code which was left over from a code restructure. Shared code between 2 tests was moved into a common method, but some code which was not cleaned up in each of these methods after the code was moved.

Modifications:
- Delete dead code in DefaultPromiseTest

Result:
Less dead code
2015-12-11 10:35:06 -08:00
Scott Mitchell
32933821bb AbstractFuture should not wrap CancellationException
Motivation:
AbstractFuture currently wraps CancellationException in a ExecutionException. However the interface of Future says that this exception should be directly thrown.

Modifications:
- Throw CancellationException from AbstractFuture.get

Result:
Interface contract for CancellationException is honored in AbstractFuture.
2015-12-11 10:24:08 -08:00
Norman Maurer
1f6b957377 Ensure we retain the original hostname when connect to a remote peer when using epoll transport.
Motivation:

We should retain the original hostname when connect to a remote peer so the user can still query the origin hostname if getHostString() is used.

Modifications:

Compute a InetSocketAddress from the original remote address and the one returned by the Os.

Result:

Same behavior when using epoll transport and nio transport.
2015-12-11 07:08:15 +01:00
Norman Maurer
f4386fb8e9 Allow to set Http2HeaderEncoder.SensitivityDetector in the Http2ConnectionHandler
Motivation:

Some times the user wants to set a Http2HeaderEncoder.SensitivityDetector when building a Http2ConnectionHandler.

Modifications:

Allow to set Http2HeaderEncoder.SensitivityDetector via builder.

Result:

More flexible building of Http2ConnectionHandler possible.
2015-12-10 14:34:04 +01:00
Norman Maurer
f31be51774 [#4505] Correctly handle whitespaces in websocket uri's.
Motivation:

If a uri contains whitespaces we need to ensure we correctly escape these when creating the request for the handshake.

Modifications:

- Correctly encode path for uri
- Add tests

Result:

Correctly handle whitespaces when doing websocket upgrade requests.
2015-12-10 13:52:42 +01:00
Norman Maurer
c6d43667d4 Add Http2HeadersEncoder.ALWAYS_SENSITIVE instance
Motivation:

We already provide a NEVER_SENSITIVE instance,we should add ALWAYS_SENSITIVE as well.

Modifications:

Add ALWAYS_SENSITIVE instance which will always return true when check for sesitive.

Result:

User can reuse code.
2015-12-10 12:59:04 +01:00
Norman Maurer
d67fea606b Use ByteBuf.*LE methods for write and read LE
Motivation:

We recently added methods to ByteBuf to directly write and read LE values. We should use these in the Snappy implementation and so reduce duplication.

Modifications:

Replace manually swapping of values with LE write and read methods of ByteBuf.

Result:

Cleaner code with less duplication.
2015-12-10 09:34:27 +01:00
Norman Maurer
088ee71222 Remove unused method in SslContext
Motivation:

We missed to remove a method in SslContext while refactored the implementation. We should remove the method to keep things clean.

Modifications:

Remove unused method.

Result:

Code cleanup.
2015-12-10 08:58:40 +01:00
Scott Mitchell
6257091d12 HttpConversionUtil does not account for COOKIE compression
Motivation:
The HTTP/2 RFC allows for COOKIE values to be split into individual header elements to get more benefit from compression (https://tools.ietf.org/html/rfc7540#section-8.1.2.5). HttpConversionUtil was not accounting for this behavior.

Modifications:
- Modify HttpConversionUtil to support compressing and decompressing the COOKIE values

Result:
HttpConversionUtil is compatible with https://tools.ietf.org/html/rfc7540#section-8.1.2.5)
Fixes https://github.com/netty/netty/issues/4457
2015-12-08 20:00:31 -08:00
Trustin Lee
2fefb2f79c Make DnsNameResolverGroup non-final and overridable
Motivation:

There's no way to override the default settings of the DnsNameResolvers
created by DnsNameResolverGroup because DnsNameResolverGroup is final.

Modifications:

- Make DnsNameResolverGroup non-final
- Add a new overridable protected method 'newResolver()' so that a user
  can override it to create an alternative DnsNameResolver instance or
  set the non-default properties

Result:

A user can configure the DnsNameResolver.
2015-12-07 19:40:01 +09:00
Anuraag Agrawal
a3bdd8c948 Don't cycle DNS servers while cycling DNS record types.
Motivation:

Each server should be checked for every record type. Currently, if there
are only two configured servers and the first is down, it is impossible
to query for IPv4 addresses because the second server is only ever
queried for type AAAA.

Modifications:

Do not cycle DNS servers while cycling DNS record types (A and AAAA)

Result:

Name resolution is less fragile when the number of available DNS servers
is 2.
2015-12-07 18:32:39 +09:00
Alex Petrov
43ebbc3fa0 Update JDK SSL Tests to use SSL Context Builder.
Motivation:

Use new / non-deprecated APIs for creating SSL Context
in tests, in order to be able to implement OpenSsl
tests with maximum code reuse.

Modifications:

Use `SslContextBuilder.(forServer|forClient)` instead
of deprecated `JdkSslServerContext` constructor.
Use `ApplicationProtocolConfig` instead of Protocol
Negotiator.
Use custom exception type for skipping tests to avoid
swallowing exceptions arising from tests.

Result:

Exceptions from tests aren't swallowed.
Using new APIs allows reusing same test code for
OpenSsl tests.
2015-12-04 11:08:57 -08:00
Trustin Lee
c1f3200c87 Fix the incorrect usage/value of 'Connection: upgrade'
Motivation:

HttpClientUpgradeHandler uses HttpHeaderNames.UPGRADE as the value of
the 'Connection' header, which is incorrect. It should use
HttpHeaderValues.UPGRADE instead (note Names vs Values.)

Also, HttpHeaderValues.UPGRADE should be 'upgrade' rather than
'Upgrade', as defined in:

- https://tools.ietf.org/html/rfc7230#section-6.7

Modifications:

- Use HttpHeaderValues.UPGRADE for a 'Connection' header
- Lowercase the value of HttpHeaderValues.UPGRADE

Result:

- Fixes #4508
- Correct behavior
2015-11-29 07:22:53 +01:00
Trustin Lee
9dd68d0c3e Fix IllegalReferenceCountException caused by HttpClientCodec.upgradeFrom()
Motivation:

On a successful protocol upgrade in HTTP, HttpClientUpgradeHandler calls
HttpClientCodec.upgradeFrom(), which removed both the HTTP encoder and
decoder from the pipeline immediately.

However, because the decoder is in the middle of the decode loop,
removing it from the pipeline immediately will cause the cumulation
buffer to be released prematurely.

This often leads to an IllegalReferenceCountException or missing first
response after the upgrade response.

Modifications:

- Remove the decoder *after* the decode loop is done

Result:

Fixes #4504
2015-11-29 07:21:08 +01:00
Trustin Lee
ef3a9b0acd Relax the sanity check in HttpClientUpgradeHandler
Motivation:

HttpClientUpgradeHandler currently throws an IllegalStateException when
the server sends a '101 Switching Protocols' response that has no
'Upgrade' header.

Some servers do not send the 'Upgrade' header on a successful protocol
upgrade and we could safely assume that the server accepted the
requested protocol upgrade in such a case, looking from the response
status code (101)

Modifications:

- Do not throw an IllegalStateException when the server responded 101
  without a 'Upgrade' header
- Note that we still check the equality of the 'Upgrade' header when it
  is present.

Result:

- Fixes #4523
- Better interoperability
2015-11-29 07:20:08 +01:00
Norman Maurer
0ec34b5f76 Fix race-condition when closing a NioSocketChannel or EpollSocketChannel
Motivation:

Fix a race-condition when closing NioSocketChannel or EpollSocketChannel while try to detect if a close executor should be used and the underlying socket was already closed. This could lead to an exception that then leave the channel / in an invalid state and so could lead to side-effects like heavy CPU usage.

Modifications:

Catch possible socket exception while try to get the SO_LINGER options from the underlying socket.

Result:

No more race-condition when closing the channel is possible with bad side-effects.
2015-11-26 22:56:00 +01:00
Luke Hutchison
4978266d52 Make cookie encoding conform better to RFC 6265 in STRICT mode.
Motivation:

- On the client, cookies should be sorted in decreasing order of path
  length. From RFC 6265:

      5.4.2. The user agent SHOULD sort the cookie-list in the following
      order:

        *  Cookies with longer paths are listed before cookies with
           shorter paths.

        *  Among cookies that have equal-length path fields, cookies with
           earlier creation-times are listed before cookies with later
           creation-times.

      NOTE: Not all user agents sort the cookie-list in this order, but
      this order reflects common practice when this document was
      written, and, historically, there have been servers that
      (erroneously) depended on this order.

  Note that the RFC does not define the path length of cookies without a
  path. We sort pathless cookies before cookies with the longest path,
  since pathless cookies inherit the request path (and setting a path
  that is longer than the request path is of limited use, since it cannot
  be read from the context in which it is written).

- On the server, if there are multiple cookies of the same name, only one
  of them should be encoded. RFC 6265 says:

      Servers SHOULD NOT include more than one Set-Cookie header field in
      the same response with the same cookie-name.

  Note that the RFC does not define which cookie should be set in the case
  of multiple cookies with the same name; we arbitrarily pick the last one.

Modifications:

- Changed the visibility of the 'strict' field to 'protected' in
  CookieEncoder.

- Modified ClientCookieEncoder to sort cookies in decreasing order of path
  length when in strict mode.

- Modified ServerCookieEncoder to return only the last cookie of a given
  name when in strict mode.

- Added a fast path for both strict mode in both client and server code
  for cases with only one cookie, in order avoid the overhead of sorting
  and memory allocation.

- Added unit tests for the new cases.

Result:

- Cookie generation on client and server is now more conformant to RFC 6265.
2015-11-26 21:41:58 +01:00
Alex Petrov
0f9492c9af Add first-class Little Endian support to ByteBuf and descendants
As discussed in	#3209, this PR adds Little Endian accessors
to ByteBuf and descendants.

Corresponding accessors were added to UnsafeByteBufUtil,
HeapByteBufferUtil to avoid calling `reverseBytes`.

Deprecate `order()`, `order(buf)` and `SwappedByteBuf`.
2015-11-26 20:30:24 +01:00
Scott Mitchell
641505a5d2 DefaultChannelConfig maxMessagesPerRead default not always set
Motivation:
ChannelMetadata has a field minMaxMessagesPerRead which can be confusing. There are also some cases where static instances are used and the default value for channel type is not being applied.

Modifications:
- use a default value which is set unconditionally to simplify
- make sure static instances of MaxMessagesRecvByteBufAllocator are not used if the intention is that the default maxMessagesPerRead should be derived from the channel type.

Result:
Less confusing interfaces in ChannelMetadata and ChannelConfig. Default maxMessagesPerRead is correctly applied.
2015-11-25 15:14:07 -08:00
nmittler
8cd259896e No HTTP/2 RST_STREAM if no prior HEADERS were sent
Motivation:

Because we flow control HEADERS frames, it's possible that an intermediate error can result in a RST_STREAM frame being sent for a frame that the other endpoint is not yet aware of. This is a violation of the spec and will either result in spammy logs at the other endpoint or broken connections.

Modifications:

Modified the HTTP/2 handler so that it only sends RST_STREAM if it has sent at least one HEADERS frame to the remote endpoint for the stream.

Result:

Fixes #4465
2015-11-25 13:46:32 -08:00
nmittler
227e67900e Fixing spammy logging for CoalescingBufferQueueTest
Motivation:

The CoalescingBufferQueueTest is somewhat relaxed with its releasing of test buffers, using safeRelease to generically deal with tests that may or may not release the buffers. SafeRelease generates logs, however, when the release fails.

Modifications:

Tightened up the individual test methods to verify that the buffers are released properly.

Result:

Fixes #4497
2015-11-25 07:16:05 -08:00
Scott Mitchell
3fa4603120 Cleanup ChannelOption.AUTO_CLOSE javadocs
Motivation:
The javadocs for ChannelOption.AUTO_CLOSE say the default is false, but the default is currently true.

Modifications:
- Make javadocs consistent with code

Result:
Less confusing docs.
2015-11-24 15:23:15 -08:00
nmittler
dbaeb3314e Allow HTTP2 frame writer to accept arbitrarily large frames
Motivation:

The encoder is currently responsible for chunking frames when writing in order to conform to max frame size. The frame writer would be a better place for this since it could perform a reuse the same promise aggregator for all the write and could also perform a single allocation for all of the frame headers.

Modifications:

Modified `DefaultHttp2FrameWriter` to perform the chunking and modified the contract in the `Http2FrameWriter` interface. Modified `DefaultHttp2ConnectionEncoder` to send give all allocated bytes to the writer.

Result:

Fixes #3966
2015-11-24 11:44:06 -08:00
nmittler
79ab756fa3 Use a single queue in UniformStreamByteDistributor
Motivation:

The UniformStreamByteDistributor currently processes all zero-length frames, regardless of add order. This means that we would always send HEADERS for all streams, possibly taking away bandwidth for streams that actually have data.

Modifications:

Empty frames are now treated the same as any other frame except that the algorithm will pop off the any empty frames at the head of the queue.

Result:

Empty frames require no extra processing.
2015-11-24 08:11:23 -08:00
Scott Mitchell
cfcee5798d Adjustable size of DefaultHeaders array
Motivation:
DefaultHeaders creates an array of size 16 for all headers. This may waste a good deal of memory if applications only have a small number of headers. This memory may be critical when the number of connections grows large.

Modifications:
- Make the size of the array for DefaultHeaders configurable

Result:
Applications can control the size of the DefaultHeaders array and save memory.
2015-11-23 15:38:08 -08:00
Scott Mitchell
a890abcf89 update pom due to alpn provided
Motiviation:
According to jetty docs the alpn-api should use the provided scope.

Modificaitons:
- change scope to provided for alpn-api
- update for new jdk

Result:
Users of Netty don't run into alpn version conflicts.
Fixes https://github.com/netty/netty/issues/4480
2015-11-23 12:52:51 -08:00
Norman Maurer
41e03adf24 Mark ChannelHandler.exceptionCaught(...) as deprected.
Motivation:

exceptionCaught(...) will only handle inbound exceptions which means it makes not much sense to have it also on ChannelOutboundHandler. Because of this we should move it to ChannelInboundHandler.

Modifications:

Add @deprecated annotation to ChannelHandler.exceptionCaught(...).

Result:

Preapre to cleanup the API in later release.
2015-11-23 09:59:22 +01:00
Trustin Lee
b6fcced64c Require RHEL/CentOS 6.7 for releasing Netty
Motivation:

6.7 is the latest stable release in RHEL/CentOS 6 line. Given that most
RHEL/CentOS users have upgraded to 6.7 via yum upgrade, we should bump
our requirement.

Modification:

s/6.6/6.7/g

Result:

'mvn release:*' must be run on RHEL/CentOS 6.7 instead of 6.6.
2015-11-21 09:56:06 +09:00
Scott Mitchell
6cb6282699 Lazy Initialization of epoll splice queue
Motivation:
AbstractEpollStreamChannel has a queue which collects splice events. Splice is assumed not to be the most common use case of this class and thus the splice queue could be initialized in a lazy fashion to save memory. This becomes more significant when the number of connections grows.

Modifications:
- AbstractEpollStreamChannel.spliceQueue will be initialized in a lazy fashion

Result:
Less memory consumption for most use cases
2015-11-20 15:09:53 -08:00
Scott Mitchell
49cd00da1c Backport of benchmark broke build
Motivation:
2a2059d976 was backported from master, and included an overriden method which does not exist in 4.1.

Modifications:
- Remove the invoker method from NoPriorityByteDistributionBenchmark

Result:
No more build error
2015-11-20 15:03:50 -08:00
Norman Maurer
7bee318fc7 Use OneTimeTask where possible to reduce object creation
Motivation:

We should use OneTimeTask where possible to reduce object creation.

Modifications:

Replace Runnable with OneTimeTask

Result:

Less object creation
2015-11-20 14:39:06 -08:00
Scott Mitchell
1f3fc983c0 DefaultPromise LateListener notification order
Motivation:
There is a notification ordering issue in DefaultPromise when the lateListener collection is in use. The ordering issue can be observed in situations where a late listener is added to a Future returned from a write operation. It is possible that this future will run after a read operation scheduled on the I/O thread, even if the late listener is added on the I/O thread. This can lead to unexpected ordering where a listener for a write operation which must complete in order for the read operation to happen is notified after the read operation is done.

Modifications:
- If the lateListener collection becomes empty, it should be treated as though it was null when checking if lateListeners can be notified immediatley (instead of executing a task on the executor)

Result:
Ordering is more natural and will not be perceived as being out of order relative to other tasks on the same executor.
2015-11-20 09:30:35 -08:00
Norman Maurer
cfa76f6326 Remove unnecessary reference to AbstractChannel from AbstractChannelHandlerContext
Motivation:

We not need to store another reference to AbstractChannel as we can access it through DefaultChannelHandlerContext.

Modifications:

Remove reference.

Result:

Cleaner code.
2015-11-20 06:41:11 -08:00
Norman Maurer
2d2e07578a Remove HashMap for lookup name / ctx from DefaultChannelPipeline to reduce memory footprint
Motivation:

If you start to have 1M+ concurrent connections memory footprint can be come a big issue. We should try to reduce it as much as possible in the core of netty.

Modifications:

- Remove HashMap that was used to store name to ctx mapping. This was only used for validation and access a handler by name. As a pipeline is not expected to be very long (like 100+ handlers) we can just walk the linked list structure to find the ctx with a given name.

Result:

Less memory footprint of the DefaultChannelPipeline.
2015-11-20 06:29:28 -08:00
Norman Maurer
edb2250d35 Store reference to IovArray in the EpollEventLoop to reduce thread local access.
Motivation:

If we have a lot of writes going on we currently need to lookup the IovArray for each Channel that does writes. This can have quite some perf overhead. We should not need to do this and just store a reference of the IovArray on the EpollEventLoop itself.

Modifications:

- Remove IoArrayThreadLocal
- Store the IoArray in the EventLoop itself

Result:

Less FastThreadLocal lookups
2015-11-20 06:07:54 -08:00
nmittler
2a2059d976 Adding UniformStreamByteDistributor
Motivation:

The current priority algorithm can yield poor per-stream goodput when either the number of streams is high or the connection window is small. When all priorities are the same (i.e. priority is disabled), we should be able to do better.

Modifications:

Added a new UniformStreamByteDistributor that ignores priority entirely and manages a queue of streams.  Each stream is allocated a minimum of 1KiB on each iteration.

Result:

Improved goodput when priority is not used.
2015-11-19 16:49:12 -08:00
Scott Mitchell
b640de2d94 Epoll Shutdown Input Exception Handling
Motivation:
If ChannelOption.ALLOW_HALF_CLOSURE is true and the shutdown input operation fails we should not propagate this exception, and instead consider this socket's read as half closed.

Modifications:
- AbstractEpollChannel.shutdownInput should not propagate exceptions when attempting to shutdown the input, but instead should just close the socket

Result:
Users expecting a ChannelInputShutdownEvent will get this event even if the socket is already shutdown, and the shutdown operation fails.
2015-11-19 16:03:10 -08:00
Dmitry Spikhalskiy
2a65ae256e [#4331] Helper methods to get charset from Content-Type header of HttpMessage
Motivation:

HttpHeaders already has specific methods for such popular and simple headers like "Host", but if I need to convert POST raw body to string I need to parse complex ContentType header in my code.

Modifications:

Add getCharset and getCharsetAsString methods to parse charset from Content-Length header.

Result:

Easy to use utility method.
2015-11-19 15:59:34 -08:00
pieteradejong
64409ad10b fixed word - issue #4469 2015-11-19 07:33:02 -08:00
nmittler
96f9b0b91b Remote flow controller incorrectly updates stream state
Motivation:

The `DefaultHttp2RemoteFlowController` does not correctly determine `hasFrame` when updating the stream state for the distributor. Adding a check to enforce `hasFrame` when `streamableBytes > 0` causes several test failures.

Modifications:

Modified `DefaultHttp2RemoteFlowController` to simplify the writing logic and to correct the bookkeeping for `hasFrame`.

Result:

The distributors are always called with valid arguments.
2015-11-18 11:32:18 -08:00
Janecek Jakub
a4ebdd0eca Fix setBytes on read-only ByteBuffer
Motivation:

The method setBytes did not work correctly because read-only ByteBuffer
does not allow access to its underlying array.

Modifications:

New case was added for ByteBuffer's that are not direct and do not have an array.
These must be handled by copying the data into a temporary array. Unit test was
added to test this case.

Result:

It is now possible to use read-only ByteBuffer as the source
for the setBytes method.
2015-11-17 22:45:37 -08:00
nmittler
8accc52b03 Forking Twitter's hpack
Motivation:

The twitter hpack project does not have the support that it used to have.  See discussion here: https://github.com/netty/netty/issues/4403.

Modifications:

Created a new module in Netty and copied the latest from twitter hpack master.

Result:

Netty no longer depends on twitter hpack.
2015-11-14 10:13:32 -08:00
Norman Maurer
2ecce8fa56 [maven-release-plugin] prepare for next development iteration 2015-11-10 22:59:33 +01:00
Norman Maurer
6a93f331d3 [maven-release-plugin] prepare release netty-4.1.0.Beta8 2015-11-10 22:50:57 +01:00
Scott Mitchell
b4b791353d AsciiString optimized hashCode
Motivation:
The AsciiString.hashCode() method can be optimized. This method is frequently used while to build the DefaultHeaders data structure.

Modification:
- Add a PlatformDependent hashCode algorithm which utilizes UNSAFE if available

Result:
AsciiString hashCode is faster.
2015-11-10 10:28:31 -08:00