Skip to content

Commit 1b4929d

Browse files
authored
Fix NettyChannelTracker race condition when tracking new channels (#904) (#906)
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 ```
1 parent 36d708e commit 1b4929d

File tree

1 file changed

+22
-15
lines changed

1 file changed

+22
-15
lines changed

driver/src/main/java/org/neo4j/driver/internal/async/pool/NettyChannelPool.java

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import io.netty.bootstrap.Bootstrap;
2222
import io.netty.channel.Channel;
2323
import io.netty.channel.ChannelFuture;
24+
import io.netty.channel.ChannelPromise;
2425
import io.netty.channel.pool.ChannelHealthChecker;
2526
import io.netty.channel.pool.FixedChannelPool;
2627

@@ -66,21 +67,27 @@ public class NettyChannelPool implements ExtendedChannelPool
6667
protected ChannelFuture connectChannel( Bootstrap bootstrap )
6768
{
6869
ListenerEvent creatingEvent = handler.channelCreating( id );
69-
ChannelFuture channelFuture = connector.connect( address, bootstrap );
70-
channelFuture.addListener( future -> {
71-
if ( future.isSuccess() )
72-
{
73-
// notify pool handler about a successful connection
74-
Channel channel = channelFuture.channel();
75-
setPoolId( channel, id );
76-
handler.channelCreated( channel, creatingEvent );
77-
}
78-
else
79-
{
80-
handler.channelFailedToCreate( id );
81-
}
82-
} );
83-
return channelFuture;
70+
ChannelFuture connectedChannelFuture = connector.connect( address, bootstrap );
71+
Channel channel = connectedChannelFuture.channel();
72+
// This ensures that handler.channelCreated is called before SimpleChannelPool calls handler.channelAcquired
73+
ChannelPromise trackedChannelFuture = channel.newPromise();
74+
connectedChannelFuture.addListener(
75+
future ->
76+
{
77+
if ( future.isSuccess() )
78+
{
79+
// notify pool handler about a successful connection
80+
setPoolId( channel, id );
81+
handler.channelCreated( channel, creatingEvent );
82+
trackedChannelFuture.setSuccess();
83+
}
84+
else
85+
{
86+
handler.channelFailedToCreate( id );
87+
trackedChannelFuture.setFailure( future.cause() );
88+
}
89+
} );
90+
return trackedChannelFuture;
8491
}
8592
};
8693
}

0 commit comments

Comments
 (0)