Spdy synchronize remove #4165
Closed
Spdy synchronize remove #4165
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…_LINGER is used. Motivation: If SO_LINGER is used shutdownOutput() and close() syscalls will block until either all data was send or until the timeout exceed. This is a problem when we try to execute them on the EventLoop as this means the EventLoop may be blocked and so can not process any other I/O. Modifications: - Add AbstractUnsafe.closeExecutor() which returns null by default and use this Executor for close if not null. - Override the closeExecutor() in NioSocketChannel and EpollSocketChannel and return GlobalEventExecutor.INSTANCE if getSoLinger() > 0 - use closeExecutor() in shutdownInput(...) in NioSocketChannel and EpollSocketChannel Result: No more blocking of the EventLoop if SO_LINGER is used and shutdownOutput() or close() is called.
Motivation: There are various places in OpenSslEngine wher we can do performance optimizations. Modifications: - Reduce JNI calls when possible - Detect finished handshake as soon as possible - Eliminate double calculations - wrap multiple ByteBuffer if possible in a loop Result: Better performance
Related commit: - a41b46f Motivation: We need a test case for the commit above.
Motivation: Sometimes it's useful to be able to create a Epoll*Channel from an existing file descriptor. This is especially helpful if you integrade some c/jni code. Modifications: - Add extra constructor to Epoll*Channel implementations that take a FileDescriptor as an argument - Make Rename EpollFileDescriptor to NativeFileDescriptor and make it public - Also ensure we obtain the correct remote/local address when create a Channel from a FileDescriptor Result: It's now possible to create a FileDescriptor and instance a Epoll*Channel via it.
…y and not causes leaks Motivation: At the moment we have two problems: - CompositeByteBuf.addComponent(...) will not add the supplied buffer to the CompositeByteBuf if its empty, which means it will not be released on CompositeByteBuf.release() call. This is a problem as a user will expect everything added will be released (the user not know we not added it). - CompositeByteBuf.addComponents(...) will either add no buffers if none is readable and so has the same problem as addComponent(...) or directly release the ByteBuf if at least one ByteBuf is readable. Again this gives inconsistent handling and may lead to memory leaks. Modifications: - Always add the buffer to the CompositeByteBuf and so release it on release call. Result: Consistent handling and no buffer leaks.
Motivation: This will avoid one unncessary method invokation which will slightly improve performance. Modifications: Instead of calling isReadable we just check for the value of readableBytes() Result: Nothing functionally speaking change.
Motivation: As we plan to have other native transports soon (like a kqueue transport) we should move unix classes/interfaces out of the epoll package so we introduce other implementations without breaking stuff before the next stable release. Modifications: Create a new io.netty.channel.unix package and move stuff over there. Result: Possible to introduce other native impls beside epoll.
Motivation: We should allow to get a ChannelOption/AttributeKey from a String. This will make it a lot easier to make use of configuration files in applications. Modifications: - Add exists(...), newInstance(...) method to ChannelOption and AttributeKey and alter valueOf(...) to return an existing instance for a String or create one. - Add unit tests. Result: Much more flexible usage of ChannelOption and AttributeKey.
Motivation: Release 4.0.25 was not usable in OSGi environments due to a simple typo. An automated test could have caught the problem even before it was committed. Modifications: This patch introduces a new artifact, osgitests, which pulls in all production artifacts (which we want to be checked for OSGi compliance). It contains only a single unit test, which runs a pax-exam container with felix OSGi. At initialization time, it scans all the artifact's dependencies, looking for things belonging to io.netty group. The container is configured to deploy those artifacts as bundles and fail if any bundle is found to be unresolved. It performs a final check to see if any bundles were tested this way, to make sure the mechanism is not completely broken. We are using wrappedBundle(), as two of our third-party dependencies do not export packages correctly -- this masks the problem, assuming that whoever deploys our artifacts depending on them will figure out how to OSGify them. Result: Simple typos and other bundle manifest errors should be caught during test phase of every build.
Motivation: Currently CORS can be configured to support a 'null' origin, which can be set by a browser if a resources is loaded from the local file system. When this is done 'Access-Control-Allow-Origin' will be set to "*" (any origin). There is also a configuration option to allow credentials being sent from the client (cookies, basic HTTP Authentication, client side SSL). This is indicated by the response header 'Access-Control-Allow-Credentials' being set to true. When this is set to true, the "*" origin is not valid as the value of 'Access-Control-Allow-Origin' and a browser will reject the request: http://www.w3.org/TR/cors/#resource-requests Modifications: Updated CorsHandler's setAllowCredentials to check the origin and if it is "*" then it will not add the 'Access-Control-Allow-Credentials' header. Result: Is is possible to have a client send a 'null' origin, and at the same time have configured the CORS to support that and to allow credentials in that combination. Conflicts: codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java
Motivation: At the moment when EmbeddedChannel is used and a ChannelHandler tries to schedule and task it will throw an UnsupportedOperationException. This makes it impossible to test these handlers or even reuse them with EmbeddedChannel. Modifications: - Factor out reusable scheduling code into AbstractSchedulingEventExecutor - Let EmbeddedEventLoop and SingleThreadEventExecutor extend AbstractSchedulingEventExecutor - add EmbbededChannel.runScheduledPendingTasks() which allows to run all scheduled tasks that are ready Result: Embeddedchannel is now usable even with ChannelHandler that try to schedule tasks.
Motivation: In the native transport we should throw a pre-instanced IOException on connection reset while reading. Modifications: Correctly throw pre-instanced IOException when ECONNRESET is received Result: Less overhead on connection reset
Motivation: When epoll LT is used and autoRead == false when entering epollIn() we need to return without reading any data. Modifications: Correctly respect autoRead == false if using epoll LT. Result: Consistent and correct behaviour.
Motivation: There is a small race in the native transport where an accept(...) may success but a later try to obtain the remote address from the fd may fail is the fd is already closed. Modifications: Let accept(...) directly set the remote address. Result: No more race possible.
…hannel goes inactive during the eventloop processing Motivation: This is a regression that was introduced as part of 6b941e9. The regression could produce an "infinity" triggering of IllegalStateException if a channel goes inactive while process the events for it. Modifications: Correctly check if the channel is still active before trigger the callbacks. Result: No more IllegalStateException
Motivation: Because of a regression sometimes accept could produce an IllegalArgumentException Modifications: Correctly respect offset when decode port and scope id. Result: No more IllegalArgumentException
Motivation: When create address from filedescriptor we may use incorrect byte order and so end up with an incorrect InetAddress. Modification: Not manually shift bytes Result: Correct address in all cases.
Motivation: Before struct's were passed per value and not pointer. This did enforce a memory copy which is not needed. Modifications: - Use "const struct....*" as replacement Result: No more unnecessary memory copies
…nnect() Motivation: EpollDragramChannel never calls fireChannelActive after connect() which is a bug. Modifications: Correctly call fireChannelActive if needed Result: Correct behaviour
…scriptor / channel Motivation: During 6b941e9 I introduced a regression that could cause an IllegalStateException. A non-proper fix was commited as part of #3443. This commit add a proper fix. Modifications: Remove FileDescriptor.INVALID and add FileDescriptor.isOpen() as replacement. Once FileDescriptor.close() is called isOpen() will return false. Result: No more IllegalStateException caused by a close channel.
…not provide Motivation: `Unpooled` javadoc's mentioned the generation of hex dump and swapping an integer's byte order, which are actually provided by `ByteBufUtil`. Modifications: Sentence moved to `ByteBufUtil` javadoc. Result: `Unpooled` javadoc is correct.
Motivation: Because of java custom UTF encoding, it was previously impossible to use nul-bytes in domain socket names, which is required for abstract domain sockets. Modifications: - Pass the encoded string byte array to the native code - Modify native code accordingly to work with nul-bytes in the the array. - Move the string encoding to UTF-8 in java code. Result: Unix domain socket addresses will work properly if they contain nul- bytes. Address encoding for these addresses changes from UTF-8-like to real UTF-8.
Motivation: When releasing unhealthy channel back to a pool we don't have to offer it since on acquire it will be discarded anyways. Also checking healthiness at release is a good idea so we don't end up having tons of unhealthy channels in the pool(unless they became unhealthy after being offered) Modifications: private SimpleChannelPool.offerIfHealthy() method added that is called from SimpleChannelPool.doReleaseChannel(). SimpleChannelPool.offerIfHealthy() offers channel back to pool only if channel is healthy. Otherwise it throws setFailure exception to the promise. Result: The pool is now much cleaner and not spammed with unhealthy channels. Added ability to choose if channel health has to be validated on release by passing boolean flag. Motivation: Depending on performance preferences and individual use cases sometimes we would like to be able force health check of a channel at release time and do not offer it back to the pool. Other times we would want to just release channel and offer it back to the pool and check health only when we try to acquire that channel from the pool. See more details here: #4077 (comment) Modifications: Future<Void> release(Channel channel, Promise<Void> promise, boolean offerHealthyOnly); The offerHealthyOnly boolean flag allows developers to choose whether to do channel validation before offering it back to pool or not. Appropriate modifications made to hierarchy of implementations of ChannelPool. offerHealthyOnly=true will force channel health to be checked before offering back to pool. offerHealthyOnly=false will ignore channel health check and will just try just offer it back to the pool offerHealthyOnly=true by default. Result: Channel health check before offer back to pool is controlled by a flag now. Code changed to satisfy checkstyle requirements. Motivation: Code needs to satisfy checkstyle requirements. Modifications: SimpleChannelPool.java:279 line split to be less then 120 characters. SimpleChannelPool.java:280:31 space added after '{' SimpleChannelPool.java:282:17 space added after '{' SimpleChannelPoolTest.java:198 - extra white space line removed. Result: Code satisfies checkstyle requirements. offerHealthyOnly is passed as a constructor parameter now. Motivation: Instead of passing offerHealthyOnly as a method parameter it is better to pass it in as SimpleChannelPool or FixedChannelPool constructor. Modifications: Redundant release method that takes offerHealthyOnly removed from ChannelPool. offerHealthyOnly parameter added to constructor for FixedChannelPool and SimpleChannelPool. Result: SimpleChannelPool and FixedChannelPool are now take offerHealthyOnly as a constructor parameter. Default behavior is: offerHealthyOnly=true. Code changed to satisfy checkstyle requirements. Motivation: Code needs to satisfy checkstyle requirements. Modifications: SimpleChannelPool.java:84: line made to be no longer then 120 characters. SimpleChannelPool.java:237: extra white space line removed. Result: Code satisfies checkstyle requirements. Tests do not need to be too copled to the code. Exception message should not be validated Motivation: We don't need our tests to be too coupled to the code. Exception type validation in tests is just good enough. Modifications: Exception validation message removed from SimpleChannelPoolTest.testUnhealthyChannelIsNotOffered() test. Result: The SimpleChannelPoolTest test is less coupled to the code now. Stack trace set to empty for UNHEALTHY_NON_OFFERED_TO_POOL. Motivation: We don't need stack trace for UNHEALTHY_NON_OFFERED_TO_POOL. Modifications: Added UNHEALTHY_NON_OFFERED_TO_POOL.setStackTrace(EmptyArrays.EMPTY_STACK_TRACE) to static init block. Result: UNHEALTHY_NON_OFFERED_TO_POOL's stack trace set to empty. Minor code re-factorings. Motivation: For better code readability we need to apply several minor code re-factorings. Modifications: javadocs true -> {@code true} offerHealthyOnly variable name changed to releaseHeathCheck <p/> -> <p> in javadocs offerHealthyOnly removed from doReleaseChannel as it not needed there. Result: Code quality is improved. Code changed to satisfy checkstyle requirements. Motivation: Code needs to satisfy checkstyle requirements. Modifications: SimpleChannelPool.java:87: line made to be no longer then 120 characters. Result: Code satisfies checkstyle requirements. Pull request needs to contain only necessary changes Motivation: The pull request should not contain unnecessary changes that are not needed as part of required functionality of pull request. Modifications: private void doReleaseChannel(final Channel channel, final Promise<Void> promise) - > private void doReleaseChannel(Channel channel, Promise<Void> promise) Result: Pull request contains less unnecessary modifications.
Motivation: On Android devices with version less than Lollipop, HarmonyJSSE is used for SSL. After completion of handshake, handshake status is NOT_HANDSHAKING instead of FINISHED. Also encrypting empty buffer after handshake should cause underflow exception and produce 0 bytes, but here it happily encrypts it causing for loop to never break Modification: Since 0 bytes should only be consumed in handshake process. Added a condition to break loop when 0 bytes are consumed and handshake status is NOT_HANDSHAKING Result: Sucessful ssl handshake on Android devices, no infinite loop now
Motivation: The StringUtil class creates a Formatter object, but does not close it. There are also a 2 utility methods which would be generally useful. Modifications: - Close the Formatter - Add length and isNullOrEmpty Result: No more resource leaks. Additional utility methods.
Motivation: The HTTP specification defines specific request-targets in https://tools.ietf.org/html/rfc7230#section-5.3. Netty does not have a way to distinguish between these differnt types, and there is currently no obvious location where these types of methods would live. Modifications: - Add methods to distinguish request-targets as defined in https://tools.ietf.org/html/rfc7230#section-5.3 Result: Common utitlity methods exist to inpsect request-targets.
Related: #3886 Motivation: We were including OSGi manifests in sources/javadoc JARs, and OSGi container treats them as correct dependencies when resolving from OBR repository, which is incorrect. Runtime fails with non-descriptive ClassNotFoundException as a result. Modifications: - Do not include the OSGi manifests in sources/javadoc JARs - Include Eclipse-related manifest entries in sources/javadoc JARs Result: Better OSGi compatibility
…ixie 76 Motivation: Hixie 76 needs special handling compared to other connection upgrade responses. Our detection code of non websocket responses did actually always use the special handling that only should be used for Hixie 76 responses. Modifications: Correctly detect connection upgrade responses which are not for websockets. Result: Be able to upgrade connections for other protocols then websockets.
… 100 Continue response is used. Motivation: Whe a 100 Continue response was written an IllegalStateException was produced as soon as the user wrote the following response. This regression was introduced by 41b0080. Modifications: - Special handle 100 Continue responses - Added unit tests Result: Fixed regression.
Motivation: The ipfilter handler does not exists in 4.0 yet. Modifications: Backport the ipfilter from 4.1 to 4.0. Result: It's possible to use the ipfilter handler in 4.0 as well.
…nactive / channelReadComplete(...) Motivation: We missed to correctly implement the handlerRemoved(...) / channelInactive(...) and channelReadComplete(...) method, this leaded to multiple problems: - Missed to forward bytes when the codec is removed from the pipeline - Missed to call decodeLast(...) once the Channel goes in active - No correct handling of channelReadComplete that could lead to grow of cumulation buffer. Modifications: - Correctly implement methods and forward to the internal ByteToMessageDecoder - Add unit test. Result: Correct behaviour
Motivation: Currently the "derived" buffer will only ever be recycled if the release call is made on the "derived" object, and the "wrapped" buffer ends up being "fully released" (aka refcount goes to 0). From my experience this is not the common use case and thus the "derived" buffers will not be recycled. Modifications: - revert #3788 Result: Less complexity, and less code to create new objects in majority of cases.
Motivation: The method implementions for setSoLinger(...) and setTrafficClass(...) were swapped by mistake. Modifications: Use the correct implementation for setSoLinger(...) and setTrafficClass(...) Result: Correct behaviour when setSoLinger(...) and setTrafficClass(...) are used with the epoll transport.
Motivation: It has been observed that passing a null argument to Unsafe.throwException can crash the JVM. Modifications: - PlatformUnsafe0.throwException should honor http://docs.oracle.com/javase/specs/jls/se8/html/jls-14.html#jls-14.18 and throw a NPE Result: No risk of JVM crashing for null argument. Fixes #4131
Motivation: The javadocs are incorrect and so give false impressions of use-pattern. Modifications: - Fix javadocs of which operations are allowed from multiple threads concurrently. - Let isEmpty() work concurrently. Result: Correctly document usage-patterns.
Motivation: Doc of ChannelGroup says, that it can be used to manage server and child channels at once. However, in DefaultChannelGroup, there is a race condition. When a server channel accepts a child, it schedules its registration on an event loop, which takes some time. If the ChannelGroup, which is supposed to close server and child channels at once, is closed after the child channel has been scheduled for registration and before this registration actually happens, this child channel is not closed and remains connected. This could lead to connection leaks. Modifications: To fix this, the DefaultChannelGroup is changed to has a closed flag. This flag is set to true, just before the close() method is actually closing channels. The add() method checks after adding a new channel, if this flag has been set to true. If yes, the new channel is closed. If not, we have the guarantee, that this channel will be closed by the ChannelGroup, because setting the closed flag to true happens-before closing any channels. This behaviour can be activated by two new constructors. The old constructors are still there and behave like before. Therefore, no existing code should be affected directly. Result: If activating this feature, the DefaultChannelGroup can be used, for managing server and child channels at once. But this activating this feature means also, that a ChannelGroup cannot be reused after calling close().
Motivation: OioSctpChannel.doReadMessages is iterating over the selected keys, and ignoring each selected key. It is not known why this is needed and no other channel implementation does this. Modifications: - Stop iterating over selected keys, and just read like other channels Result: No unnecessary iteration in OioSctpChannel.doReadMessages. Fixes #3884
Motivation: When try to get SO_LINGER from a fd that is closed an Exception is thrown. We should only try to get SO_LINGER if the fd is still open otherwise an Exception is thrown that can be ignored anyway. Modifications: First check if the fd is still open before try to obtain SO_LINGER setting when get the closeExecutor. This is also the same that we do in the NIO transport. Result: No more exception when calling unsafe.close() on a channel that has a closed file descriptor.
Motivation: A new netty-tcnative bugfix release was released. Modifications: Upgrade version. Result: Using latest netty-tcnative version
Motivation: Sometimes it is useful to disable recycling completely if memory constraints are very tight. Modifications: Allow to use -Dio.netty.recycler.maxCapacity.default=0 to disable recycling completely. Result: It's possible to disable recycling now.
…cutor. Motivation: for debugging and metrics reasons its sometimes useful to be able to get details of the the Thread that powers a SingleThreadEventExecutor. Modifications: - Expose ThreadProperties - Add unit test. Result: It's now possible to get details of the Thread that powers a SingleThreadEventExecutor.
Motivation: The alpn / npn dependency versions are dependent on java version. If a java version 1.8+ is used that is not explicitly listed in the pom file then ALPN tests will fail because the java 1.7 version of alpn will be loaded by out pom file. Modifications: - Ensure there is a latest version to fall back up for npn 1.7+ - Ensure there is a latest version to fall back upon from alpn 1.8+ Result: Build can complete despite having a newer jdk which is not listed in our pom file.
Motivation: When a LocalChannel write operation occurs, the promise associated with the write operation is marked successful when it is added to the peer's queue, but before the peer has actually received the data. If the promise callback closes the channel then a race condition exists where the close event may occur before the data is delivered. We should preserve ordering of events. Modifications: - LocalChannel should track when a write is in progress, and if a close operation happens make sure the peer gets all pending read operations. Result: LocalChannel preserves order of operations. Fixes #4118
Motivation: #4143 addressed a few ordering issues but an ordering issue still remained if the Promise for a write completes, and a listener of that promise does a write on a peer channel. The ordering was subject to how potentially 2 different executors would run a task, but it should be coordinated such that the first write is read first. Modifications: - Keep track of the finishPeerRead task run on the executor if necessary and ensure it completes before current channel read occurs Result: Ordering of events for echo type situations is preserved.
This reverts commit 4feafc4.
Motivation: As all methods in the ChannelHandler are executed by the same thread there is no need to use synchronized. Modifications: Remove synchronized keyword. Result: No more unnessary synchronized in SpdySessionHandler.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.