timoninmaxim commented on a change in pull request #8206:
URL: https://github.com/apache/ignite/pull/8206#discussion_r488673289
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java
##########
@@ -169,8 +167,10 @@
// No-op.
}
- for (ClientChannelHolder hld : channels)
- hld.closeChannel();
+ if (channels.get() != null) {
+ for (ClientChannelHolder hld: channels.get())
Review comment:
There is no race conditions. As channels are set within synchronized
method `initChannelHolders`. The discussed code is placed within `close` that
is synchronized too. Also channels is never reference to null after it
initialized, so this check is enough.
For other accessors to the `channels` field:
1. `rollCurrentChannel` invokes after channels is initialized. Also it
guards with `curChannelsGuard.writeLock`, so it is safe the reference directly;
2. `applyOnDefaultChannel` can lead to NPE in case `initConnection` is not
invoked. So I've just lazy initialization in the method, so there is a
guarantee that channels are initialized. In our case this method call just
returns fast;
3. `initAllChannelsAsync` are invoked after channels are initialized. Also
it uses `ArrayList.iterator()` to iterate through channels. Under the hood it
just creates Iterator object that. We change `channels` by reference switch so,
this iterator is safe to use.
4. `channelsInit` is checks channels for null and then initialize them is it
needed. So it's safe too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
