Skip to content
Permalink
Branch: master
Commits on Apr 7, 2020
  1. Promote Single.TerminalSignalConsumer to top level interface (#1008)

    Scottmitch committed Apr 7, 2020
    Motivation:
    TerminalSignalConsumer for Completable and Publisher is a top level
    type, but Single has an internal type for the same purpose. This makes
    it more challenging to expose utilities classes in a consistent manner
    and also may lead to naming collisions.
    
    Modifications:
    - Move Single.TerminalSignalConsumer to a top level interface and rename
    to SingleTerminalSignalConsumer to avoid naming collisions.
    - Expose RunnableTerminalSignalConsumer and re-use it in
    BeforeFinallyOnHttpResponseOperator.
    
    Result:
    More consistent type definition scheme for TerminalSignalConsumer and
    SingleTerminalSignalConsumer and ability to expose conversion/utility
    methods outside the scope of the Source.
  2. Change when* operators to before* instead of after* (#1005)

    Scottmitch committed Apr 7, 2020
    Motivation:
    The when* operators provide call backs to take action when that action
    isn't sensitive to execution order. The current implementation of the
    when* operators uses after* semantics, despite the order being explicitly
    undefiend by the API. The after* semantics may result in user code
    executing in unexpected times, espeically when local state is involved.
    For example tracing filters utilize AsyncContext and trigger the state
    cleanup on the response payload Publisher termination. This filter must
    be appended first in order for the trace information to be initialized
    correctly and if any subsequent filters apply a payload Publisher
    transformation with after* semantics the terminal methods will be
    invoked after the tracing Scope has been closed. If subsequent operators
    use before* semantics they will not be subject to this ordering issue.
    
    Modifications:
    - Change Publisher, Single, Completable when* operators to use before*
    semantics instead of after* semantics.
    
    Result:
    when* operators use before* semantics to reduce the risk that local
    state managed outside the scope of the operator flow is cleaned up when
    user code executes.
  3. Rename BeforeFinallyOnHttpResponseOperator to BeforeFinallyHttpOperat…

    Scottmitch committed Apr 7, 2020
    …or (#1007)
    
    Motivation:
    BeforeFinallyOnHttpResponseOperator include the 'On' terminology which
    doesn't make sense in this context as the event name doesn't include
    'On' (e.g. onSubscribe). The 'Response' component can also be dropped to
    simplify the naming.
    
    Modifications:
    - Rename BeforeFinallyOnHttpResponseOperator to
    BeforeFinallyHttpOperator
    
    Result:
    More concise naming of BeforeFinallyHttpOperator.
  4. AsyncContextInMemoryScopeManager to restore previous Scope on close() (

    Scottmitch committed Apr 7, 2020
    …#1004)
    
    Motivation:
    AsyncContextInMemoryScopeManager currently doesn't restore the previous
    Scope when the current Scope is closed. This behavior is provided for
    ThreadLocalScope and allows users to use try-with-resources type
    constructs to create sub-spans more easily.
    
    Modifications:
    - AsyncContextInMemoryScopeManager to save/restore the previous Scope
    when a new Scope is activated
    - AsyncContextInMemoryScopeManager to remove the currentScope() hack so
    lifetime of Scopes is more narrowly/correctly constrainted. This means
    that filter ordering is important and use of after* operators should be
    done with care!
    
    Result:
    AsyncContextInMemoryScopeManager has more consistent behavior with
    ThreadLocalScope, and lifetimes of Scopes are more narrowly/correctly
    constrained.
Commits on Apr 3, 2020
  1. Completable#concatWith(Completable) remove atomic operation (#1000)

    Scottmitch committed Apr 3, 2020
    Motivation:
    Completable#concatWith(Completable) currently uses an atomic operation to transition subscribe() from the first Completable to the second. However this is done in the contex of a Subscriber and the events should be sequenced in a [serial](https://github.com/reactive-streams/reactive-streams-jvm/blob/v1.0.3/README.md#1.3) fashion.
    
    Modifications:
    - Completable#concatWith(Completable) to use a regular variable instead of volatile/atomic operation to switch subscribers
    
    Result:
    Less atomic operations in Completable#concatWith(Completable).
  2. Work around SpotBugs JDK11 bug, make code more robust (#998)

    Scottmitch committed Apr 3, 2020
    Motivation:
    SpotBugs has a bug impacting builds on JDK11 spotbugs/spotbugs#756. This also hinted at an area that could be made more robust.
    
    Modifications:
    - Ignore SpotBugs warning due to bug
    - Improve robustness of handling null element from iterator
    
    Result:
    SpotBugs no longer fails on JDK11 builds.
Commits on Apr 2, 2020
  1. Replace ConcurrentSubscription with DelayedCancellable when synchrono…

    Scottmitch committed Apr 2, 2020
    …us request(Long.MAX_VALUE) (#993)
    
    Motivation:
    There are some operators which convert from Publisher to Completable or
    Single. Since the demand for Completable and Single is implicit on
    subscribe(..) these operators need to simulate the demand in
    onSubscribe. In order to prevent concurrent access on the Subscription
    with the downstream Subscriber and the synchornous requestN call it is
    wrapped in a ConcurrentSubscription. However ConcurrentSubscription has
    to coordinate between two methods and is inheritly more complicated than
    DelayedCancellable. We can use DelayedCancellable which is a simpler
    operation at the cost of requesting demand before propagating
    synchronous cancel operation. This isn't perceived as a deal breaker
    because demand is implicit anyways, and cancel is best effort.
    
    Modifications:
    - When converting from Publisher to Completable or Single use
    DelayedCancellable instead of ConcurrentSubscription
    
    Result:
    Less complicated and less expensive conversion from Subscription to
    Cancellable when converting from Publisher to Single or Completable.
Commits on Apr 1, 2020
  1. Update to Gradle 6.3 (#992)

    Scottmitch committed Apr 1, 2020
    Motivation:
    Gradle 6.3 supports JDK 14 https://docs.gradle.org/6.3/release-notes.html
Commits on Mar 31, 2020
  1. Add support for SameSite attribute in Cookies (#989)

    Scottmitch committed Mar 31, 2020
    Motivation:
    There is a revision of the HTTP cookies RFC (6265) in review which adds
    a new SameSite attribute
    https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-05#section-5.3.7.
    
    Modifications:
    - Add the SameSite attribute support to the SetCookies related APIs
    
    Result:
    HttpSetCookies now support the SameSite attribute.
Commits on Mar 26, 2020
  1. gRPC gradle plugin write script to buildDir (#984)

    Scottmitch committed Mar 26, 2020
    Motivation:
    servicetalk-grpc-gradle-plugin was writing the executable script files
    to the temp directory and not cleaning them up. There was also a bug in
    a variable substitution which lead to compile errors.
    
    Modifications:
    - Write to the buildDir instead of temp, and don't rewrite the file on
    every build if it already exists since the version information is in the
    file name.
    - Fix the variable naming bug
    
    Result:
    No more temp files for servicetalk-grpc-gradle-plugin and one less
    variable naming bug.
  2. NettyChannelPublisher cancel active subscriber should terminate (#976)

    Scottmitch committed Mar 26, 2020
    Motivation:
    NettyChannelPublisher allows for resubscribes, and delivers queued data to the new Subscriber. If the previous Subscriber consumed some data and cancelled this may lead to the new Subscriber receiveing partial data from the last request.
    
    Modifications:
    - NettyChannelPublisher should discard any pending data, and deliver an error to new Subscribers if a previously active Subscriber cancels.
    
    Result:
    Resubscribes to NettyChannelPublisher won't get partial data for previous requests, like in the case of client pipelinining.
  3. gRPC gradle plugin rework (#983)

    Scottmitch committed Mar 26, 2020
    Motivation:
    0ebe48e divided the
    servicetalk-grpc-gradle plugin into two files:
    1. an executable script
    2. an uber jar with the plugin logic
    The executable script assumed the uber jar would be co-located in the
    same directory as the uber jar, but that isn't the case in gradle
    caches. This means the plugin may fail to execute outside of the maven
    m2 repository structure.
    
    Modifications:
    - Instead of publishing a static script for each platform which assumes
    a co-located uber jar, dynamically generate the executable script
    depending upon where the uber jar is resolved from for the local build.
    
    Result:
    servicetalk-grpc-gradle works with gradle cache directory structure and
    local development.
Commits on Mar 25, 2020
  1. gRPC protoc script pushd not found (#982)

    Scottmitch committed Mar 25, 2020
    Motivation:
    The gRPC protoc unix script is expected to execute in a /bin/sh environment, but it depends upon pushd/popd which may not be available in /bin/sh. This may lead to a 'pushd: not found' error.
    
    Modifications:
    - Avoid using pushd in the unix script and manually save/restore the directory
    
    Result:
    gRPC protoc script can execute in limited /bin/sh environments without
    failure.
  2. WriteStreamSubscriber aggregate Netty Promise return true (#978)

    Scottmitch committed Mar 25, 2020
    Motivation:
    WriteStreamSubscriber uses a shared Promise instance for all writes via AllWritesPromise. Netty's Promise API has try* methods which require a boolean response to know if they promise state changed as a result, which is difficult to implement with a shared promise. AllWritesPromise currently returns false if the promise has been completed in ways other than Netty's Promise try* APIs which Netty treats as an error an may take an expensive path to raise an issue like logging stack traces.
    
    Modifications:
    - Return true instead of false to avoid additional work in Netty due to perceived failure. This isn't strictly correct from the API perspective but in practice the asynchronous contorl flow is still completed and Netty thinks everything is fine.
    
    Result:
    Less expensive error reporting in the event a write promise has been completed before Netty drains write queues when an error is detected.
Commits on Mar 24, 2020
  1. Initial support for windows development (#977)

    Scottmitch committed Mar 24, 2020
    Motivation:
    ServiceTalk currently doesn't build on windows. This can be limiting for
    folks who are comfortable with that development environment.
    
    Modifications:
    - grpc-protoc project needs refactoring of the plugin in order to
    execute on unix and windows platforms. The distribution is now
    consistent with a script as the executable and co-located uber jar
    containing the plugin logic.
    - ignore/modify tests that failed when running locally on windows, there
    will be followup PRs for other tests that sporadically fail.
    
    Result:
    ServiceTalk can now be built on windows.
Commits on Mar 23, 2020
  1. Fix ChannelSetTest async issues (#979)

    Scottmitch committed Mar 23, 2020
    Motivation:
    ChannelSetTest is flaky because it doens't wait for events to occurr that it validates.
    
    Modifications:
    - Introduce TestCollectingCompletableSubscriber which allows for tests to wait for specific conditions before delivering events and validation.
    - Use this in ChannelSetTest
    
    Result:
    ChannelSetTest is more reliable.
Commits on Mar 14, 2020
  1. Update gradlew to 6.2.2 (#966)

    Scottmitch committed Mar 14, 2020
    Motivation:
    Update to the latest version of gradlew and update dependencies.
    
    Modifications:
    - Use spotbugs 4.0.2 plugin and 4.0.0 executable
    - Remove explicit guava compile dependency
    
    Result:
    Project now uses gradlew 6.2.2.
Commits on Mar 11, 2020
  1. Publisher#range operator (#961)

    Scottmitch committed Mar 11, 2020
    Motivation:
    The canonocal way to do an asynchornous for lop with integer indexes in reactive streams is to use a range() operator. We currently don't have a range operator.
    
    Modifications:
    - Add Publisher#range(int begin, int end)
    - Add Publisher#range(int begin, int end, int stride)
    
    Result:
    For loops with integer indexes can be done directly with Publisher#range(..)
Commits on Mar 9, 2020
Commits on Mar 6, 2020
Commits on Mar 3, 2020
Commits on Feb 25, 2020
  1. WriteStreamSubscriber and WriteListener cleanup (#923)

    Scottmitch committed Feb 25, 2020
    Motivation:
    The WriteListener interface has grown over time and its naming conventions have
    become less clear. For example "closeGracefully" implies that some action is
    expected to be taken to account for pending writes, but in reality this is just
    a notification that the transport's outbound side has been closed and should be
    interpreted accordingly by the implementation.
    The CloseHandler introduced a new class and public static variable which is
    intended for use in the http/2.0 codec. However the CloseHandler isn't protocol
    specific and the naming of this class implies an inverted dependency
    relationship.
    
    Modifications:
    - Rename WritableListener to ChannelOutboundListener. Rename method names on
    this interface to indicate they are notification of channel events and
    implementations can react however they need to.
    - Rename H2ProtocolHandler to ProtocolOutboundCloseEventHandler to be more
    agnositc to specific protocols which may use this class and associated public
    static singleton instance.
    - DefaultNettyConnectionTest writes a static buffer, but doesn't duplicate() on
    each use. The test now duplicates the buffer to avoid shared indexes.
    - Avoid forcing error logs from WriteSingleSubscriber because the async
    control flow has already been completed and the additional noise/overhead of the
    log may not be necessary.
    
    Result:
    More clarity around ChannelOutboundListener interfaces and implementations.
Commits on Jan 17, 2020
  1. Remove LegacyBlockingSubscriber (#918)

    Scottmitch committed Jan 17, 2020
    Motivation:
    LegacyBlockingSubscriber was intended for removal but still had some remaining
    utility methods used to orchestrate sequential blocking test execution. This
    functionality can be replaced by a TestCollectingPublisherSubscriber.
    
    Modifications:
    - Remove LegacyBlockingSubscriber
    - Add TestCollectingPublisherSubscriber which can be used in combination with
    TestPublisherSubscriber or independently.
    
    Result:
    Legacy code removed, more integrated sequential blocking test utilities.
Commits on Jan 10, 2020
Commits on Dec 20, 2019
  1. Disallow space after header name and colon (#901)

    Scottmitch committed Dec 20, 2019
    Motivation:
    https://tools.ietf.org/html/rfc7230#section-3.2.4 specifies that spaces are
    prohibited between the end of the header-name and the colon. Our header parsing
    is currently liberal and allows a space. This may leave us vulnerable to HTTP
    request smuggling.
    
    Modifications:
    - HttpObjectDecoder should prevent white space before the colon when paring
      header-name field
    
    Result:
    HTTP/1.x header parsing follows
    https://tools.ietf.org/html/rfc7230#section-3.2.4
Commits on Dec 18, 2019
Commits on Dec 17, 2019
  1. HTTP/1.x and HTTP/2.0 decoder Buffer visibility and data corruption (#…

    Scottmitch committed Dec 17, 2019
    …898)
    
    Motivation:
    ServiceTalk attempts to effectivley disable Netty's reference counting by using
    a strategy similar to Netty's `UnreleasableByteBuf` to short circit operations
    which attempt to modify the reference count, and therefore prevent the reference
    count from reaching zero and prematruley freeing memory. ServiceTalk's
    ByteBuf instances will therefore always have a `refCnt() == 1`. However Netty's
    ByteToMessageDecoder uses `refCnt() > 1` to determine if the cumulation ByteBuf
    maybe shared, and if not it may compact data in place. This compacting data in
    place may result in a corrupted view of data from the perspective of offloaded
    threads in ServiceTalk, and for example result in failures to decode HTTP/2 data
    such as gRPC frames.
    
    ServiceTalk's `ByteToMessageDecoder` uses Netty's
    `ByteToMessageDecoder#MERGE_CUMULATOR` to manage accumulation of data. As of
    Netty 4.1.43.Final this may result in writing into the `cumulation` ByteBuf such
    that a reallocation opearation occurs. This reallocation operation is not done
    in a thread safe way and this may result in visibility issues when referencing
    the same `Buffer` object from another thread (e.g. when offloading is enabled).
    This may result in the applicaiton thread observing an empty `Buffer` if the
    reference to the newly allocated memory is visibile before the copy operation is
    visible.
    
    Modifications:
    - All `ByteBuf` objects generated by `ServiceTalkBufferAllocator` should have a
    `refCnt() > 1` to avoid any optimizations which may assume exclusive ownership
    of a `ByteBuf` in Netty. This will prevent corrupted content for HTTP/2.x and
    gRPC use cases. This may result in more reallocation/copy operations in Netty's
    `ByteToMessageDecoder` and there is a Netty PR
    netty/netty#9877 intended to improve this behavior.
    - ServiceTalk's `ByteToMessageDecoder` should not use
    `ByteToMessageDecoder#MERGE_CUMULATOR` and instead use a strategy similar to
    netty/netty#9877 to do the accumulation of bytes. It is
    also unecessary to attempt to compact the buffer after accumulation has been
    attempted, as any compaction can be handled in the accumulation.
    
    Result:
    No more decode related visibility issues for HTTP/1.x and no more content
    corruption and visibility issues for HTTP/2.0 / gRPC.
Commits on Dec 13, 2019
  1. Remove redundant state from ProtoBufSerializationProvider (#897)

    Scottmitch committed Dec 13, 2019
    Motivation:
    ProtoBufSerializationProvider has extra state stateReadHeader to track if the
    gRPC header is being read, but this can be inferred based upon existing state.
    
    Modifications:
    - remove stateReadHeader from ProtoBufSerializationProvider and use lengthOfData
    instead
    
    Result:
    Less state required by ProtoBufSerializationProvider.
Commits on Dec 12, 2019
  1. Revert 0c4280d (#894)

    Scottmitch committed Dec 12, 2019
    Motivation:
    0c4280d attempted to avoid reclaiming of old
    memory when resized for cases where the memory maybe shared by multiple threads.
    However modifying a Buffer accross mulitiple threads may lead to visibility
    issues where the reference to the new array maybe visible before the memory is
    copied into the array, and in general requires external synchornization.
    
    Modificiations:
    - Revert the freeDirect(ByteBuffer) override introduced in
    0c4280d
    
    Result:
    We can reclaim intermediate memory from resize operations again.
Commits on Dec 11, 2019
  1. DirectByteBuf implementations should not force free memory (#893)

    Scottmitch committed Dec 11, 2019
    Motivation:
    ServiceTalk doesn't expose reference counting and relies upon GC for memory
    reclamation. Buffer objects are resizable an in the event a buffer is resized
    (e.g. a write operation needs to expand the buffer) this will result in
    allocating new memory and Netty will attempt to deallocate the old memory.
    Applications may be multi-threaded and it is possible another thread will still
    have a reference to this old memory location due to visibility constraints. We
    should prevent Netty from force-freeing this old memory and instead rely upon GC
    to avoid undefined behavior.
    
    Modifications:
    - ServiceTalk DirectByteBuf implementations to override freeDirect(ByteBuffer)
      and do nothing.
    
    Result:
    Less risk for resize operation resulting in referencing freed memory.
Commits on Oct 30, 2019
  1. ReactiveStreams offloading tests to create single executor for test (#…

    Scottmitch committed Oct 30, 2019
    …831)
    
    Motivation:
    We have seen test errors related to the Executor being shutdown during the ReactiveStreams TCK offloading tests. We currently create a new executor before/after each method. The executor member variable is shared state and the isolation with testng needs more investigation. However it doesn't seem necessary to recreate the executor for each method invocation.
    
    Modifications:
    - Make the executor variable static, and only initialize/tear down once per test class
    
    Result:
    Less executor creation/destruction during RS TCK tests. Related to #829
Older
You can’t perform that action at this time.