Skip to content

Conversation

@injectives
Copy link
Contributor

@injectives injectives commented Jun 4, 2021

Cherry-pick of #904

This update fixes an issue when SimpleChannelPool may invoke the channelAcquired method before the channelCreated method is called. Since it violates NettyChannelTracker contract, it may result in runtime failures.

See the following for details:

@startuml
T1 -> SimpleChannelPool: acquireHealthyFromPoolOrNew
activate SimpleChannelPool

SimpleChannelPool -> NettyChannelPool: connectChannel
activate NettyChannelPool

NettyChannelPool -> ChannelFuture: addListener
activate ChannelFuture

ChannelFuture --> NettyChannelPool:
deactivate ChannelFuture

NettyChannelPool --> SimpleChannelPool:
deactivate NettyChannelPool

T2 -> ChannelFuture: setSuccess
activate ChannelFuture

ChannelFuture -> ChannelFuture: done
activate ChannelFuture

ChannelFuture --> ChannelFuture:
deactivate ChannelFuture

SimpleChannelPool -> ChannelFuture: isDone
activate ChannelFuture

ChannelFuture --> SimpleChannelPool:
deactivate ChannelFuture

alt isDone case
    SimpleChannelPool -> NettyChannelTracker: channelAcquired
    activate NettyChannelTracker

    NettyChannelTracker --> SimpleChannelPool:
    deactivate NettyChannelTracker
end

SimpleChannelPool --> T1:
deactivate SimpleChannelPool

ChannelFuture -> NettyChannelTracker: channelCreated
activate NettyChannelTracker

NettyChannelTracker -> ChannelFuture:
deactivate NettyChannelTracker

ChannelFuture -> T2:
deactivate ChannelFuture
@enduml

…o4j#904)

This update fixes an issue when `SimpleChannelPool` may invoke the `channelAcquired` method before the `channelCreated` method is called. Since it violates `NettyChannelTracker` contract, it may result in runtime failures.

See the following for details:
```
@startuml
T1 -> SimpleChannelPool: acquireHealthyFromPoolOrNew
activate SimpleChannelPool

SimpleChannelPool -> NettyChannelPool: connectChannel
activate NettyChannelPool

NettyChannelPool -> ChannelFuture: addListener
activate ChannelFuture

ChannelFuture --> NettyChannelPool:
deactivate ChannelFuture

NettyChannelPool --> SimpleChannelPool:
deactivate NettyChannelPool

T2 -> ChannelFuture: setSuccess
activate ChannelFuture

ChannelFuture -> ChannelFuture: done
activate ChannelFuture

ChannelFuture --> ChannelFuture:
deactivate ChannelFuture

SimpleChannelPool -> ChannelFuture: isDone
activate ChannelFuture

ChannelFuture --> SimpleChannelPool:
deactivate ChannelFuture

alt isDone case
    SimpleChannelPool -> NettyChannelTracker: channelAcquired
    activate NettyChannelTracker

    NettyChannelTracker --> SimpleChannelPool:
    deactivate NettyChannelTracker
end

SimpleChannelPool --> T1:
deactivate SimpleChannelPool

ChannelFuture -> NettyChannelTracker: channelCreated
activate NettyChannelTracker

NettyChannelTracker -> ChannelFuture:
deactivate NettyChannelTracker

ChannelFuture -> T2:
deactivate ChannelFuture
@enduml
```
@injectives injectives requested review from bigmontz, dotStart and michael-simons and removed request for dotStart and michael-simons June 4, 2021 10:52
Copy link
Contributor

@bigmontz bigmontz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧑‍🚒

@injectives injectives merged commit e30888a into neo4j:4.2 Jun 4, 2021
@injectives injectives deleted the feature/channel-tracker-fix-4-2 branch June 4, 2021 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants