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
```
Copy link
Contributor

@michael-simons michael-simons left a comment

Choose a reason for hiding this comment

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

Thanks for back porting it. I double checked that the NettyChannelTracker is on the same state as well.

@injectives injectives merged commit 1b4929d into neo4j:4.1 Jun 7, 2021
@injectives injectives deleted the feature/channel-tracker-fix-4-1 branch June 24, 2021 09:02
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