Skip to content

Fix NettyChannelTracker race condition when tracking new channels #906

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 7, 2021

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