Motivation:
We need to ensure we call ctx.flush() before closing the actual channel when an handshake failure took place. If we miss to do so we may not send all pending data to the remote peer which also include SSL alerts.
Modifications:
Ensure we call ctx.flush() before ctx.close() on a handshake error.
Result:
All pending data (including SSL alerts) are written to the remote peer on a handshake error.
Motivation:
We need to ensure we do all checks inside of the try / catch block so we free native memory that was allocated in the constructor of the super class in a timely manner.
Modifications:
Move all checks inside of the try block.
Result:
Correctly release native memory (and not depend on the finalizer) when a check in the constructors fails
Motivation:
In commit acbca192bd we changed to have our native operations which either gall getsockopt or setsockopt throw IOExceptions (to be more specific we throw a ClosedChannelException in some cases). Unfortunally I missed to also do the same for getSoError() and missed to add throws IOException to the native methods.
Modifications:
- Correctly throw IOException from getSoError()
- Add throws IOException to native methods where it was missed.
Result:
Correct declaration of getSoError() and other native methods.
Conflicts:
transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollChannel.java
Motivation:
A user may use a private key which is encrypted with an empty password. Because of this we should only handle a null password in a special way.
Modifications:
- Correctly handle private key that is encrypted with empty password.
- Make OpenSsl*Context implementions consistent in terms of initialization in the constructor.
Result:
Correctly support private key that is encrypted with empty password.
Motivation:
We also need to add synchronization when access fields to ensure we see the latest updates.
Modifications:
Add synchronization when read fields that are written concurrently.
Result:
Ensure correct visibility of updated.
Motivation:
If SO_LINGER is set to 0 the EPOLL transport will send a FIN followed by a RST. This is not consistent with the behavior of the NIO transport. This variation in behavior can cause protocol violations in streaming protocols (e.g. HTTP) where a FIN may be interpreted as a valid end to a data stream, but RST may be treated as the data is corrupted and should be discarded.
https://github.com/netty/netty/issues/4170 Claims the behavior of NIO always issues a shutdown when close occurs. I could not find any evidence of this in Netty's NIO transport nor in the JDK's SocketChannel.close() implementation.
Modifications:
- AbstractEpollChannel should be consistent with the NIO transport and not force a shutdown on every close
- FileDescriptor to keep state in a consistent manner with the JDK and not allow a shutdown after a close
- Unit tests for NIO and EPOLL to ensure consistent behavior
Result:
EPOLL is capable of sending just a RST to terminate a connection.
Motivation:
We had some double spacing in the methods which should be removed to keep things consistent.
Modifications:
Remove redundant spaces.
Result:
Cleaner / consistent coding style.
Motivation:
My previous commit b88a980482 introduced a flawed unit test,
that executes an assertion in a different thread than the test thread.
If this assertion fails, the test doesn't fail.
Modifications:
Replace the assertion by a proper workaround.
Result:
More correct unit test
Motivation:
To be consistent with the JDK we should ensure our native methods throw a ClosedChannelException if the Channel was previously closed. This will then be wrapped in a ChannelException as usual. For all other errors we continue to just throw a ChannelException directly.
Modifications:
Ensure getsockopt and setsockopt will throw a ClosedChannelException if the channel was closed before, on other errors we throw a ChannelException as before diretly.
Result:
Consistent with the NIO Channel implementations.
Motivation:
Circular assignment of arenas to thread caches can lead to less than optimal
mappings in cases where threads are (frequently) shutdown and started.
Example Scenario:
There are a total of 2 arenas. The first two threads performing an allocation
would lead to the following mapping:
Thread 0 -> Arena 0
Thread 1 -> Arena 1
Now, assume Thread 1 is shut down and another Thread 2 is started. The current
circular assignment algorithm would lead to the following mapping:
Thread 0 -> Arena 0
Thread 2 -> Arena 0
Ideally, we want Thread 2 to use Arena 1 though.
Presumably, this is not much of an issue for most Netty applications that do all
the allocations inside the eventloop, as eventloop threads are seldomly shut down
and restarted. However, applications that only use the netty-buffer package
or implement their own threading model outside the eventloop might suffer from
increased contention. For example, gRPC Java when using the blocking stub
performs some allocations outside the eventloop and within its own thread pool
that is dynamically sized depending on system load.
Modifications:
Implement a linear scan algorithm that assigns a new thread cache to the arena
that currently backs the fewest thread caches.
Result:
Closer to ideal mappings between thread caches and arenas. In order to always
get an ideal mapping, we would have to re-balance the mapping whenever a thread
dies. However, that's difficult because of deallocation.
Motivation:
We want to allow the use of an uber jar that contains shared dynamic libraries for all platforms (including fedora).
Modifications:
Modified OpenSsl to try and load the fedora library if the OS is Linux and the platform specified library fails before using the default lib.
Result:
True uber support.
Motivation:
PromiseAggregator's API allows for the aggregate promise to complete before the user is done adding promises. In order to support this use case the API structure would need to change in a breaking manner.
Modifications:
- Deprecate PromiseAggregator and subclasses
- Introduce PromiseCombiner which corrects these issues
Result:
PromiseCombiner corrects the deficiencies in PromiseAggregator.
Motivation:
We should always first notify the promise before trigger an event through the pipeline to be consistent.
Modifications:
Ensure we notify the promise before fire event.
Result:
Consistent behavior
Motivation:
EpollServerSocketConfig.isFreebind() throws an exception when called.
Modifications:
Use the correct getsockopt arguments.
Result:
No more exception when call EpollServerSocketConfig.isFreebind()
Motivation:
TCP_MD5 is only supported by SocketChannels so remove it from EpollServerChannelConfig which is generic.
Modifications:
Remove invalid code.
Result:
Remove invalid / dead code.
Motivation:
A custom SecurityManager may prevent calling File.exists() and so throw a SecurityException in the static init block of NetUtil.
Modifications:
Correctly catch the exception and so allow to static init NetUtil.
Result:
Allow static init method of NetUtil to work even with custom SecurityManager.
Motivation:
The statistic counters PoolArena.(allocationsTiny|allocationsSmall) are
not protected by a per arena lock, but by a per size class lock. Thus,
two concurrent allocations of different size (class) could lead to a
race and ultimately to wrong statistics.
Modifications:
Use a thread-safe LongCounter instead of a plain long data type.
Result:
Fewer data races.
Motivation:
PendingWriteQueue should guard against re-entrant writes once
removeAndFailAll() is run.
Modifications:
removeAndFailAll() should repeat until the queue is finally empty.
Result:
assertEmpty() will always hold.
Motivation:
The current HttpPostMultipartRequestDecoder can decode multipart/form-data parts with a Content-Type that specifies a charset. When this charset is invalid the Charset.forName() throws an unchecked UnsupportedCharsetException. This exception is not catched by the decoder. It should actually be rethrown as an ErrorDataDecoderException, because the developer using the API would expect this validation failure to be reported as such.
Modifications:
Add a catch block for UnsupportedCharsetException and rethrow it as an ErrorDataDecoderException.
Result:
UnsupportedCharsetException are now rethrown as ErrorDataDecoderException.
Motivation:
When using the native transport have support for TCP_DEFER_ACCEPT or / and TCP_QUICKACK can be useful.
Modifications:
- Add support for TCP_DEFER_ACCEPT and TCP_QUICKACK
- Ad unit tests
Result:
TCP_DEFER_ACCEPT and TCP_QUICKACK are supported now.
Conflicts:
transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollChannelOption.java
Motivation:
We should guard users from using Unsafe methods from outside the EventLoop if not designed to do so.
Modifications:
Add asserts
Result:
Easier for users to detect miss-use.
Motivation:
DefaultChannelHandlerContext calls a utility method validatePromise which may throw if the arguments are not valid. If this method throws then the message will not be released.
Modifications:
- If an exception is thrown the message should be released
Result:
No more leak in DefaultChannelHandlerContext.write
Motivation:
See #3321
Modifications:
1. Add CharsetUtil.encoder/decoder() methods
2. Deprecate CharsetUtil.getEncoder/getDecoder() methods
Result:
Users can use new CharsetUtil.encoder/decoder() to specify error actions
Motivation:
For on tests we expected a ConnectTimeoutException but used the default timeout of 10 seconds. This slows down testing.
Modifications:
Use connect timeout of 1 second in unit test.
Result:
Faster execution of unit test.
Motivation:
See #3172
Modifications:
https://github.com/netty/netty-build/pull/6 added a junit timeout listener to the netty-build project. This patch just set it up.
Result:
If a test is set the timeout parameter using junit's @Test(timeout = ...) and the timeout is triggered, a full stack trace dump will be outputted and also output the deadlocks if any.
Motivation:
It will be easier to support websockets in server application by using WebSocketServerProtocolHandshakeHandler class and not reinvent its functionality. But currently it handles all http requests as if they were websocket handshake requests.
Modifications:
Check if http request path is equals to adjusted websocket path.
Fixed example of websocket server implementation.
Result:
WebSocketServerProtocolHandshakeHandler handles only websocket handshake requests.
Motivation:
If the input buffer is empty we should not have decodeLast(...) call decode(...) as the user may not expect this.
Modifications:
- Not call decode(...) in decodeLast(...) if the input buffer is empty.
- Add testcases.
Result:
decodeLast(...) will not call decode(...) if input buffer is empty.
Motivation:
Utility methods in ByteBufUtil to writeUtf8 and writeAscii expect a buffer to already be allocated. If the user does not have a buffer allocated they have to know details of the encoding in order to know the size of the buffer to allocate.
Modifications:
- Add writeUtf8 and writeAscii which take a ByteBufAllocator and allocate a ByteBuf of the correct size for the user
Result:
ByteBufUtil methods which are easier to use if the user doesn't already have a ByteBuf.
Motivation:
See https://github.com/netty/netty-build/issues/5
Modifications:
Add xml-maven-plugin to check indentation and fix violations
Result:
pom.xml will be checked in the PR build
Motivation:
Each call of ByteBuf.getByte(int) method does boundary checking. This can be eliminated by using ByteBuf.forEachByte(ByteProcessor) method and ByteProcessor.FIND_LF processor.
Modifications:
Find end of line with ByteProcessor.FIND_LF
Result:
A little better performance of LineBasedFrameDecoder.
Motivation:
Build warning message is shown when building netty project. Due to missing paxexam version below[1] warning message is shown when building netty project.
Modifications:
Added paxexam plugin version into root pom
Result:
paxexam related warning will not be displayed when building the project.
[1]. #4909
Motivation:
We want to allow the use of an uber jar that contains the shared libraries for all platforms.
Modifications:
Modified OpenSsl to first check for a platform-specific lib before using the default lib.
Result:
uber support.
Motivation:
See #3746.
Modifications:
Fork SpscLinkedQueue and SpscLinkedAtomicQueue from JCTools based on 7846450e28
Result:
Add SpscLinkedQueue and SpscLinkedAtomicQueue and apply it in LocalChannel.
Motivation:
If the Future that the PromiseNotifier is listening to is cancelled, it does not propagate the cancel to all the promises it is expected to notify.
Modifications:
- If the future is cancelled then all the promises should be cancelled
- Add a UnaryPromiseNotifier if a collection of promises is not necessary
Result:
PromiseNotifier propagates cancel events to all promises
Motivation:
Depending on the actual CertificateException we should set the correct alert type so it will be sent back to the remote peer and so make it easier for them to fix it.
Modification:
Correctly set the alert and not always just use a general alert.
Result:
It's easier for the remote peer to fix the problems.
Motivation:
See #4882
Modification:
isInputShutdown, isOutputShutdown and shutdownOutput methods moved from io.netty.channel.socket.SocketChannel to a new interface io.netty.channel.socket.DuplexChannel.
io.netty.channel.unix.DomainSocketChannel now extends DuplexChannel
Methods implementing DuplexChannel moved from EpollSocketChannel to AbstractEpollStreamChannel.
Result:
Possible to call shutdownOutput on EpollDomainSocketChannel
Motivation:
Commit 108dc23cab introduced a workaround due to a JDK crash when GCM cipher was used during an unwrap operation. Attempting to reproduce this issue with the latest JDK (1.8.0_72-b15) demonstrate that this issue no longer exists while it can be reliably reproduced on earlier JDKs (1.8.0_25-b17 and earlier)
Modifications:
- Remove the copy-to-heap-buffer workaround for JDK engine
Result:
Fixes https://github.com/netty/netty/issues/3256
Motivation:
ByteToMessageDecoder must ensure that read components of the CompositeByteBuf can be discard by default when discardSomeReadBytes() is called. This may not be the case before as because of the default maxNumComponents that will cause consolidation.
Modifications:
Ensure we not do any consolidation to actually be abel to discard read components
Result:
Less memory usage and allocations.
Motivation:
netty-tcnative-1.1.33.Fork was released, we should upgrade. Also we should skip renegotiate tests if boringssl is used because boringssl does not support renegotiation.
Modifications:
- Upgrade to netty-tcnative-1.1.33.Fork13
- Skip renegotiate tests if boringssl is used.
Result:
Use newest version of netty-tcnative and be able to build if boringssl is used.
Motivation:
b714297a44 introduced ChannelInputShutdownEvent support for HttpObjectDecoder. However this should have been added to the super class ByteToMessageDecoder, and ByteToMessageDecoder should not propegate a channelInactive event through the pipeline in this case.
Modifications:
- Move the ChannelInputShutdownEvent handling from HttpObjectDecoder to ByteToMessageDecoder
- ByteToMessageDecoder doesn't call ctx.fireChannelInactive() on ChannelInputShutdownEvent
Result:
Half closed events are treated more generically, and don't get translated into a channelInactive pipeline event.
Motiviation:
We should ensure that handlerAdded(...) and handlerRemoved(...) is always called from the EventExecutor that also invokes the other methods of the ChannelHandler. Also we need to ensure we always call handlerAdded(...) before any other method can be calld to ensure correct ordering.
Motifications:
- Ensure that the right thread is used to call the methods
- Ensure correct ordering
- Add tests
Result:
Respect the thread-model for handlerAdded(...) and handlerRemoved(...) and preserve correct ordering in all cases.
