Commit Graph

6277 Commits

Author SHA1 Message Date
Trustin Lee
563bf24424 Do not use SLF4J/Logback API directly
Motivation:

TrafficShapingHandlerTest uses Logback API directly, which is
discouraged.  Also, it overrides the global default log level, which
silences the DEBUG messages from other tests.

Modifications:

Remove the direct use of Logback API

Result:

The tests executed after TrafficShapingHandlerTest logs their DEBUG
messages correctly.
2014-12-17 10:15:52 +09:00
Scott Mitchell
93194c6595 Parent pom inconsistent between baselines
Motivation:
ALPN version updates revealed an inconsistency visible by defaulting to npn when alpn was expected.

Modifications:
Default to ALPN.

Result:
Build and unit tests should pass.
2014-12-16 19:27:22 -05:00
Scott Mitchell
08f7b24d6a Java ALPN provider version update
Motivation:

There was a bug in the Java ALPN library we are using.  A new version was released to fix this bug and we should update our pom.xml to use the new version.

Modifications:

Update pom.xml to use new ALPN library.

Result:

Newer versions of JDK (1.7_u71, 1.7_u72, 1.8_u25) have the bug fixed.
2014-12-16 16:26:28 -05:00
Trustin Lee
4f7632a853 Fix checkstyle 2014-12-16 20:34:18 +09:00
Trustin Lee
98a2bb62f5 Log detailed information about renegotiation and traffic
Motivation:

We need more information to understand why SocketSslEchoTest fails
sporadically in the CI machine.

Modifications:

- Refactor SocketSslEchoTest so that it is easier to retrieve the
  information about renegotiation and the current progress

Result:

We will get more information when the test fails.
2014-12-16 17:45:21 +09:00
Trustin Lee
c7b407e288 Compress the heap dump after tests are finished
Motivation:

Tests sometimes time out because it took too long to compress the
generated heap dump.

Modifications:

- Move the compression logic to a new method 'compressHeapDumps()'
- Call TestUtils.compressHeapDumps() at the end of the tests, so that
  the tests do not fail because of timeout

Result:

JUnit reports the real cause of the test failure instead of timeout
exception.
2014-12-16 16:16:17 +09:00
Leonardo Freitas Gomes
9ee75126eb Motivation: Sonar points out an equals comparison, where the types compared are different and don't share any common parent http://clinker.netty.io/sonar/drilldown/issues/io.netty:netty-parent:master?severity=CRITICAL#
Modifications:
Converted AsciiString into a String by calling toString() method before comparing with equals(). Also added a unit-test to show that it works.

Result:
Major violation is gone. Code is correct.
2014-12-16 07:17:31 +01:00
zcourts
bd6d0f3fd5 ensure getRawQuery is not null before appending
Motivation:

without this check then given a URI with path /path the resulting URL will be /path?null=

Modifications:

check that getRawQuery doesn't return null and only append if not

Result:

urls of the form /path will not have a null?= appended
2014-12-16 06:53:29 +01:00
Trustin Lee
8d4a97e05b Generate non-test JAR for netty-testsuite
Motivation:

So far, we generated and deployed test JARs to Maven repositories. The
deployed JAR had the classifier 'test-jar'.  The test JAR is consumed by
transport-native-epoll as a test dependency.

The problem is, when netty-transport-native-epoll pulls the test JAR as
a dependency, that Maven resolves its transitive dependencies at
'compile' and 'runtime' scope only, which is incorrect.

I was bitten by this problem recently while trying to add a new
dependency to netty-testsuite.  Because I added a new dependency at the
'test' scope, the new dependency was not pulled transitively by
transport-native-epoll and caused an unexpected build failure.

- d6160208c3
- bf77bb4c3a

Modifications:

- Move all classes in netty-testsuite from src/test to src/main
- Update the 'compile' scope dependencies of netty-testsuite
- Override the test directory configuration properties of the surefire
  plugin
- Do not generate the test JAR anymore
- Update the dependency of netty-transport-native-epoll

Result:

It is less error-prone to add a new dependency to netty-testsuite.
2014-12-15 09:18:08 +09:00
Trustin Lee
5dd48cf3fe Fix build errors due to missing dependency 2014-12-14 21:29:52 +09:00
Trustin Lee
cbbf5eb96b Compress the heap dump generated by TestUtils.dump()
Motivation:

It takes too long to download the heap dump from the CI server.

Modifications:

Compress the heap dump as much as possible.

Result:

When heap dump is generated by certain test failure, the generated heap
dump file is about 3 times smaller than before, although the compression
time will increase the build time when the test fails.
2014-12-14 12:01:38 +09:00
ysammy
0bc0851569 Fix documentation for ChannelHandlerContext#fireChannelReadComplete
Motivation:
Fix a minor documentation bug in
ChannelHandlerContext#fireChannelReadComplete.

Modifications:
ChannelHandlerContext#fireChannelReadComplete no longer references an
incorrect method in its javadoc.

Results:
Documentation is correct.
2014-12-12 18:43:06 +01:00
Norman Maurer
73c0f85d63 Remove bottleneck while create InetSocketAddress in native transport
Motivation:

Everytime a new connection is accepted via EpollSocketServerChannel it will create a new EpollSocketChannel that needs to get the remote and local addresses in the constructor. The current implementation uses new InetSocketAddress(String, int) to create these. This is quite slow due the implementation in oracle and openjdk.

Modifications:

Encode all needed informations into a byte array before return from jni layer and then use new InetSocketAddress(InetAddress, int) to create the socket addresses. This allows to create the InetAddress via a byte[] and so reduce the overhead, this is done either by using InetAddress.getByteAddress(byte[]) or by Inet6Address.getByteAddress(String, byte[], int).

Result:

Reduce performance overhead while accept new connections with native transport
2014-12-12 18:16:23 +01:00
Trustin Lee
dfd0cc5ea2 Add more assertions related with TLS renegotiation 2014-12-12 18:01:30 +09:00
Trustin Lee
e7cf1dcb98 Test TLS renegotiation with explicit cipher suite change
Motivation:

So far, our TLS renegotiation test did not test changing cipher suite
during renegotiation explicitly.

Modifications:

- Switch the cipher suite during renegotiation

Result:

We are now sure the cipher suite change works.
2014-12-12 17:49:16 +09:00
Trustin Lee
6ee069ffb4 Add log messages when dump starts
.. to make it easier to find the right dump file for a test when there
are multiple dump files.
2014-12-12 11:55:27 +09:00
Trustin Lee
e72b2235fb Make sure to notify handshake success even if SSLEngine is closed
Related:

e9685ea45a

Motivation:

SslHandler.unwrap() does not evaluate the handshake status of
SSLEngine.unwrap() when the status of SSLEngine.unwrap() is CLOSED.

It is not correct because the status does not reflect the state of the
handshake currently in progress, accoding to the API documentation of
SSLEngineResult.Status.

Also, sslCloseFuture can be notified earlier than handshake notification
because we call sslCloseFuture.trySuccess() before evaluating handshake
status.

Modifications:

- Notify sslCloseFuture after the unwrap loop is finished
- Add more assertions to SocketSslEchoTest

Result:

Potentially fix the regression caused by:
- e9685ea45a
2014-12-12 11:55:27 +09:00
Trustin Lee
3cd94822f5 Generate heap and thread dump when some tests fail
Motivation:

We have a few sporadic test failures which are only easily reproduceable
in our CI machine.  To get more information about the failure, we need
heap and full thread dump at the moment of failure.

Modifications:

- Add TestUtils.dump() method to dump heap and threads
- Modify SocketGatheringWriteTest and SocketSslEchoTest to call
  TestUtils.dump() on failure

Result:

We get more information about the test failure.
2014-12-12 10:35:05 +09:00
Norman Maurer
12c9ce7f67 Allow to lazy create a DefaultFileRegion from a File
Motivation:

We only provided a constructor in DefaultFileRegion that takes a FileChannel which means the File itself needs to get opened on construction. This has the problem that if you want to write a lot of Files very fast you may end up with may open FD's even if they are not needed yet. This can lead to hit the open FD limit of the OS.

Modifications:

Add a new constructor to DefaultFileRegion which allows to construct it from a File. The FileChannel will only be obtained when transferTo(...) is called or the DefaultFileRegion is explicit open'ed via open() (this is needed for the native epoll transport)

Result:

Less resource usage when writing a lot of DefaultFileRegion.
2014-12-11 12:04:17 +01:00
Norman Maurer
182c91f06c Ensure buffer is not released when call array() / memoryAddress()
Motivation:

Before we missed to check if a buffer was released before we return the backing byte array or memoryaddress. This could lead to JVM crashes when someone tried various bulk operations on the Unsafe*ByteBuf implementations.

Modifications:

Always check if the buffer is released before all to return the byte array and memoryaddress.

Result:

No more JVM crashes because of released buffers when doing bulk operations on Unsafe*ByteBuf implementations.
2014-12-11 11:30:31 +01:00
Trustin Lee
d5a24d4f6c Make SslHandler work when autoRead is turned off
Related: #2958

Motivation:

SslHandler currently does not issue a read() request when it is
handshaking. It makes a connection with autoRead off stall, because a
user's read() request can be used to read the handshake response which
is invisible to the user.

Modifications:

- SslHandler now issues a read() request when:
  - the current handshake is in progress and channelReadComplete() is
    invoked
  - the current handshake is complete and a user issued a read() request
    during handshake
- Rename flushedBeforeHandshakeDone to flushedBeforeHandshake for
  consistency with the new variable 'readDuringHandshake'

Result:

SslHandler should work regardless whether autoRead is on or off.
2014-12-11 17:57:28 +09:00
Trustin Lee
1dc1831abf Add SslHandler.renegotiate()
Related: #3125

Motivation:

We did not expose a way to initiate TLS renegotiation and to get
notified when the renegotiation is done.

Modifications:

- Add SslHandler.renegotiate() so that a user can initiate TLS
  renegotiation and get the future that's notified on completion
- Make SslHandler.handshakeFuture() return the future for the most
  recent handshake so that a user can get the future of the last
  renegotiation
- Add the test for renegotiation to SocketSslEchoTest

Result:

Both client-initiated and server-initiated renegotiations are now
supported properly.
2014-12-10 18:59:50 +09:00
Trustin Lee
298e7af647 Trigger channelWritabilityChanged() later to avoid reentrance
Related: #3212

Motivation:

When SslHandler and ChunkedWriteHandler exists in a pipeline together,
it is possible that ChunkedWriteHandler.channelWritabilityChanged()
invokes SslHandler.flush() and vice versa. Because they can feed each
other (i.e. ChunkedWriteHandler.channelWritabilityChanged() ->
SslHandler.flush() -> ChunkedWriteHandler.channelWritabilityChanged() ->
..), they can fall into an inconsistent state due to reentrance (e.g.
bad MAC record at the remote peer due to incorrect ordering.)

Modifications:

- Trigger channelWritabilityChanged() using EventLoop.execute() when
  there's a chance where channelWritabilityChanged() can cause a
  reentrance issue
- Fix test failures caused by the modification

Result:

Fix the handler reentrance issues related with a
channelWritabilityChanged() event
2014-12-10 18:40:26 +09:00
Trustin Lee
9ff234abed Call ctx.flush() at least once in ChunkedWriteHandler.flush()
Related: #3219

Motivation:

ChunkedWriteHandler.flush() does not call ctx.flush() when channel is
not writable. This can be a problem when other handler / non-Netty
thread writes messages simultaneously, because
ChunkedWriteHandler.flush() might have no chance to observe
channel.isWritable() returns true and thus the channel is never flushed.

Modifications:

- Ensure that ChunkedWriteHandler.flush() calls ctx.flush() at least
  once.

Result:

A stall connection issue, that occurs when certain combination of
handlers exist in a pipeline, has been fixed. (e.g. SslHandler and
ChunkedWriteHandler)
2014-12-09 18:17:46 +09:00
Daniel Bevenius
2a426b3d44 Fixing minor typo in FastThreadLocal javadoc. 2014-12-08 13:52:54 +01:00
Frederic Bregier
9b2fede68e Fix AbstractDiskHttpData int conversion from long
Motivations:
The chunkSize might be oversized after comparison (size being > of int
capacity) if file size is bigger than an integer.

Modifications:
Change it to long.

Result:
There is no more int oversized.

Same fix for 4.1 and Master
2014-12-08 07:17:46 +01:00
Trustin Lee
3957a88a94 Make PendingWriteQueue.recycle() update its state before triggering an event
Related: #3212

Motivation:

PendingWriteQueue.recycle() updates its data structure after triggering
a channelWritabilityChanged() event. It causes a rare corruption such as
double free when channelWritabilityChanged() method accesses the
PendingWriteQueue.

Modifications:

Update the state of PendingWriteQueue before triggering an event.

Result:

Fix a rare double-free problem
2014-12-07 23:27:33 +09:00
Trustin Lee
1ef6f14734 Trigger exceptionCaught() when VoidChannelPromise fails
Related: #3190

Motivation:

When an outbound handler method raises an exception, its promise is
marked as failed.  If the promise is done already, the exception is
logged.

When the promise is void, exceptionCaught() must be triggered to notify
a user. However, ChannelHandlerInvokerUtil simply swallows it.

Modifications:

Do not swallow an exception when the promise is void.

Result:

A user who uses a void promise for an outbound operation will be
notified on failure.
2014-12-07 16:06:55 +09:00
Trustin Lee
3dbca4a9e2 Fire channelRead() event immediately in OIO message channels
Related: #3189

Motivation:

OIO transport implementations block for at most 1 second to wait for
additional messages (or accepted connections).

However, because AbstractOioMessageChannel defers the channelRead()
events for the messages read so far until there's nothing to read up to
maxMessagesPerRead, any read operation will be followed by a 1-second
delay.

Modifications:

Fire channelRead() events as soon as doRead() returns so that there is
no 1 second delay between the actual read and the channelRead() event.

Result:

No more weird 1-second delay
2014-12-07 12:13:26 +09:00
Scott Mitchell
8206cc6e14 Headers set/add/contains timeMillis methods
Motivation:
The new Headers interface contains methods to getTimeMillis but no add/set/contains variants.  These should be added for consistency.

Modifications:
- Add three new methods: addTimeMillis, setTimeMillis, containsTimeMillis to the Headers interface.
- Add a new method to the Headers.ValueConverter interface: T convertTimeMillis(long)
- Bring these new interfaces up the class hierarchy

Result:
All Headers classes have setters/getters for timeMillis.
2014-12-06 22:40:28 +09:00
Trustin Lee
23db94f5a1 Fix Java 6 compatibility issue in DnsNameResolver
Related: #3173

Motivation:

DnsNameResolver was using InetSocketAddress.getHostString() which is
only available since Java 7.

Modifications:

Use InetSocketAddress.getHostName() in lieu of getHostString() when the
current Java version is less than 7.

Result:

DnsNameResolver runs fine on Java 6.
2014-12-06 22:31:44 +09:00
Trustin Lee
7f92771496 Fix a bug where Recycler's capacity can increase beyond its maximum
Related: #3166

Motivation:

When the recyclable object created at one thread is returned at the
other thread, it is stored in a WeakOrderedQueue.

The objects stored in the WeakOrderedQueue is added back to the stack by
WeakOrderedQueue.transfer() when the owner thread ran out of recyclable
objects.

However, WeakOrderedQueue.transfer() does not have any mechanism that
prevents the stack from growing beyond its maximum capacity.

Modifications:

- Make WeakOrderedQueue.transfer() increase the capacity of the stack
  only up to its maximum
- Add tests for the cases where the recyclable object is returned at the
  non-owner thread
- Fix a bug where Stack.scavengeSome() does not scavenge the objects
  when it's the first time it ran out of objects and thus its cursor is
  null.
- Overall clean-up of scavengeSome() and transfer()

Result:

The capacity of Stack never increases beyond its maximum.
2014-12-06 17:58:31 +09:00
Greg Gibeling
a79466769f Lazily check for root, avoids unnecessary errors & resources
Motivation:

io.netty.util.internal.PlatformDependent.isRoot() depends on the IS_ROOT field which is filled in during class initialization. This spawns processes and consumes resources, which are not generally necessary to the complete functioning of that class.

Modifications:

This switches the class to use lazy initialization this field inside of the isRoot() method using double-checked locking (http://en.wikipedia.org/wiki/Double-checked_locking).

Result:

The first call to isRoot() will be slightly slower, at a tradeoff that class loading is faster, uses fewer resources and platform errors are avoided unless necessary.
2014-12-05 09:17:06 +01:00
Daniel Norberg
54a39a94ac example: memcache: fix set command
Motivation:

The example MemcacheClient set command doesn't work.

Modifications:

Fill the extras field buffer with zeros so that it gets written to the
request payload.

Result:

The example MemcacheClient set command works.
2014-12-05 08:59:23 +01:00
Trustin Lee
46576fd2ff Fix a race condition where handler is removed before unregistration
Related: #3156

Motivation:

Let's say we have a channel with the following pipeline configuration:

  HEAD --> [E1] H1 --> [E2] H2 --> TAIL

when the channel is deregistered, the channelUnregistered() methods of
H1 and H2 will be invoked from the executor thread of E1 and E2
respectively. To ensure that the channelUnregistered() methods are
invoked from the correct thread, new one-time tasks will be created
accordingly and be scheduled via Executor.execute(Runnable).

As soon as the one-time tasks are scheduled,
DefaultChannelPipeline.fireChannelUnregistered() will start to remove
all handlers from the pipeline via teardownAll(). This process is
performed in reversed order of event propagation. i.e. H2 is removed
first, and then H1 is removed.

If the channelUnregistered() event has been passed to H2 before H2 is
removed, a user does not see any problem.

If H2 has been removed before channelUnregistered() event is passed to
H2, a user will often see the following confusing warning message:

  An exceptionCaught() event was fired, and it reached at the tail of
  the pipeline. It usually means the last handler in the pipeline did
  not handle the exception.

Modifications:

To ensure that the handlers are removed *after* all events are
propagated, traverse the pipeline in ascending order before performing
the actual removal.

Result:

A user does not get the confusing warning message anymore.
2014-12-05 16:09:00 +09:00
Trustin Lee
bd8a5bc9b3 Add missing @Override annotation 2014-12-04 20:53:14 +09:00
Trustin Lee
70a91c72c4 Fix checkstyle 2014-12-04 18:40:50 +09:00
Trustin Lee
bf58f871c3 Overall clean-up of the initial SniHandler/DomainNameMapping work
- Parameterize DomainNameMapping to make it useful for other use cases
  than just mapping to SslContext
- Move DomainNameMapping to io.netty.util
- Clean-up the API documentation
- Make SniHandler.hostname and sslContext volatile because they can be
  accessed by non-I/O threads
2014-12-04 18:23:07 +09:00
Trustin Lee
96d596802b Fix dependency issues with hamcrest
Motivation:

We use 3 (!) libraries to build mock objects - easymock, mockito, jmock.
Mockito and jMock pulls in the different versions of Hamcrest, and it
conflicts with the version pulled by jUnit.

Modifications:

- Replace mockito-all with mockito-core to avoid pulling in outdated
  jUnit and Hamcrest
- Exclude junit-dep when pulling in jmock-junit4, because it pulls an
  outdated Hamcrest version
- Pull in the hamcrest-library version used by jUnit explicitly

Result:

No more dependency hell that results in NoSuchMethodError during the
tests
2014-12-04 17:59:15 +09:00
Sun Ning
8f77c80795 Added support for SSL Server Name Indication.
Motivation:

When we need to host multiple server name with a single IP, it requires
the server to support Server Name Indication extension to serve clients
with proper certificate. So the SniHandler will host multiple
SslContext(s) and append SslHandler for requested hostname.

Modification:

* Added SniHandler to host multiple certifications in a single server
* Test case

Result:

User could use SniHandler to host multiple certifcates at a time.
It's server-side only.
2014-12-03 11:03:15 +01:00
Ronald Chen
bbe880f5ea Rocumented decoder pitfalls to avoid mistakes found in [#3184] 2014-12-01 20:25:56 +01:00
Sam Young
a37c4ad7f4 Add @SafeVarargs to PromiseAggregator#add and PromiseNotifier#(...) https://github.com/netty/netty/issues/3147
Motivation:

8fbc513 introduced stray warnings in callsites of
PromiseAggregator#add and PromiseNotifier#(...).

Modifications:

This commit adds the @SafeVarargs annotation to PromiseAggregator#add
and PromiseNotifier#(...). As Netty is built with JDK7, this is a
recognized annotation and should not affect runtime VM versions 1.5 and
1.6.

Result:

Building Netty with JDK7 will no longer produce warnings in the
callsites mentioned above.
2014-12-01 19:59:41 +01:00
Trustin Lee
46ef370ee1 Copy the resolver configuration when cloning Bootstrap
Motivation:

Bootstrap.clone() does not copy the resolver configuration.

Modifications:

Copy the resolver configuration when cloning.

Result:

Bug fixed
2014-12-01 19:50:12 +09:00
Jay
2769ad428a Check the bindFuture before writing a DNS query
Related: #3149

Motivation:

DnsQueryContext, using the DatagramChannel bound in DnsNameResolver,
blindly writes to the channel without checking the bind future for
success.

Modifications:

Check the bindFuture before writing a DNS query to a DatagramChannel

Result:

Bug fixed
2014-12-01 19:46:18 +09:00
Frank Barber
f4d3f81d6c Prevent channel re-registration from firing channelActive
Motivation:

AbstractUnsafe considers two possibilities during channel registration. First,
the channel may be an outgoing connection, in which case it will be registered
before becoming active. Second, the channel may be an incoming connection in,
which case the channel will already be active when it is registered. To handle
the second case, AbstractUnsafe checks if the channel is active after
registration and calls ChannelPipeline.fireChannelActive() if so.  However, if
an active channel is deregistered and then re-registered this logic causes a
second fireChannelActive() to be invoked. This is unexpected; it is reasonable
for handlers to assume that this method will only be invoked once per channel.

Modifications:

This change introduces a flag into AbstractUnsafe to recognize if this is the
first or a subsequent registration. ChannelPipeline.fireChannelActive() is only
possible for the first registration.

Result:

ChannelPipeline.fireChannelActive() is only called once.
2014-11-30 19:51:30 +01:00
Ronald Chen
e1273147fa replaced broken &lt with < and same for gt 2014-11-29 19:33:50 +01:00
Graham Edgecombe
d33a80da26 Use Triple DES in JdkSslContext cipher suite list.
Motivation:

JdkSslContext used SSL_RSA_WITH_DES_CBC_SHA in its cipher suite list.
OpenSslServerContext used DES-CBC3-SHA in the same place in its cipher suite
list, which is equivalent to SSL_RSA_WITH_3DES_EDE_CBC_SHA.

This means the lists were out of sync. Furthermore, using
SSL_RSA_WITH_DES_CBC_SHA is not desirable as it uses DES, a weak cipher. Triple
DES should be used instead.

Modifications:

Replace SSL_RSA_WITH_DES_CBC_SHA with SSL_RSA_WITH_3DES_EDE_CBC_SHA in
JdkSslContext.

Result:

The JdkSslContext and OpenSslServerContext cipher suite lists are now in sync.
Triple DES is used instead of DES, which is stronger.
2014-11-27 08:15:34 +01:00
Trustin Lee
88aa9e7c4a Remove or de-prioritize RC4 from default cipher suites
Motivation:

RC4 is not a recommended cipher suite anymore, as the recent research
reveals, such as:

- http://www.isg.rhul.ac.uk/tls/

Modifications:

- Remove most RC4 cipher suites from the default cipher suites
- For backward compatibility, leave RC4-SHA, while de-prioritizing it

Result:

Potentially safer default
2014-11-25 17:17:20 +09:00
Trustin Lee
247d5b1bd9 Fix awful naming 2014-11-22 07:46:59 +09:00
Trustin Lee
040c340f76 Add back IntObjectMap.values(Class<V>)
Motivation:

Although the new IntObjectMap.values() that returns Collection is
useful, the removed values(Class<V>) that returns an array is also
useful. It's also good for backward compatibility.

Modifications:

- Add IntObjectMap.values(Class<V>) back
- Miscellaneous improvements
  - Cache the collection returned by IntObjectHashMap.values()
  - Inspector warnings
- Update the IntObjectHashMapTest to test both values()

Result:

- Backward compatibility
- Potential performance improvement of values()
2014-11-22 07:42:14 +09:00