dlmarion commented on code in PR #5662:
URL: https://github.com/apache/accumulo/pull/5662#discussion_r2180961932
##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -320,12 +320,13 @@ protected ServerAddress getThriftServerAddress() {
return thriftServer;
}
- protected void updateAdvertiseAddress(HostAndPort thriftBindAddress) {
- if (advertiseAddress == null) {
- advertiseAddress = thriftBindAddress;
- } else if (!advertiseAddress.hasPort()) {
- advertiseAddress =
- HostAndPort.fromParts(advertiseAddress.getHost(),
thriftBindAddress.getPort());
+ protected synchronized void updateAdvertiseAddress(HostAndPort
thriftBindAddress) {
+ final HostAndPort address = advertiseAddress.get();
+ if (address == null) {
+ advertiseAddress.compareAndSet(null, thriftBindAddress);
+ } else if (!address.hasPort()) {
+ advertiseAddress.compareAndSet(address,
+ HostAndPort.fromParts(address.getHost(),
thriftBindAddress.getPort()));
Review Comment:
I think `advertiseAddress` either needs to be an AtomicReference or volatile
as two different threads could call `updateAdvertiseAddress` and
`getAdvertiseAddress`. The `synchronized` modifier isn't for the memory
effects, it's so that it's not called concurrently by different threads.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
