- Avoid ConcurrentModificationException when `getMachines()` by deep clone the machine map - Enhance the code to avoid potential ConcurrentModificationException - Add test casesmaster
@@ -99,6 +99,11 @@ | |||||
<groupId>com.alibaba</groupId> | <groupId>com.alibaba</groupId> | ||||
<artifactId>fastjson</artifactId> | <artifactId>fastjson</artifactId> | ||||
</dependency> | </dependency> | ||||
<dependency> | |||||
<groupId>junit</groupId> | |||||
<artifactId>junit</artifactId> | |||||
<scope>test</scope> | |||||
</dependency> | |||||
</dependencies> | </dependencies> | ||||
<build> | <build> | ||||
@@ -15,15 +15,16 @@ | |||||
*/ | */ | ||||
package com.taobao.csp.sentinel.dashboard.discovery; | package com.taobao.csp.sentinel.dashboard.discovery; | ||||
import java.util.HashSet; | |||||
import java.util.Optional; | import java.util.Optional; | ||||
import java.util.Set; | import java.util.Set; | ||||
import java.util.TreeSet; | |||||
import java.util.concurrent.ConcurrentHashMap; | |||||
public class AppInfo { | public class AppInfo { | ||||
private String app = ""; | private String app = ""; | ||||
private Set<MachineInfo> machines = new TreeSet<MachineInfo>(); | |||||
private Set<MachineInfo> machines = ConcurrentHashMap.newKeySet(); | |||||
public AppInfo() { | public AppInfo() { | ||||
} | } | ||||
@@ -40,8 +41,13 @@ public class AppInfo { | |||||
this.app = app; | this.app = app; | ||||
} | } | ||||
public synchronized Set<MachineInfo> getMachines() { | |||||
return machines; | |||||
/** | |||||
* Get the current machines. | |||||
* | |||||
* @return a new copy of the current machines. | |||||
*/ | |||||
public Set<MachineInfo> getMachines() { | |||||
return new HashSet<>(machines); | |||||
} | } | ||||
@Override | @Override | ||||
@@ -49,7 +55,7 @@ public class AppInfo { | |||||
return "AppInfo{" + "app='" + app + ", machines=" + machines + '}'; | return "AppInfo{" + "app='" + app + ", machines=" + machines + '}'; | ||||
} | } | ||||
public synchronized boolean addMachine(MachineInfo machineInfo) { | |||||
public boolean addMachine(MachineInfo machineInfo) { | |||||
machines.remove(machineInfo); | machines.remove(machineInfo); | ||||
return machines.add(machineInfo); | return machines.add(machineInfo); | ||||
} | } | ||||
@@ -16,6 +16,7 @@ | |||||
package com.taobao.csp.sentinel.dashboard.discovery; | package com.taobao.csp.sentinel.dashboard.discovery; | ||||
import java.util.Date; | import java.util.Date; | ||||
import java.util.Objects; | |||||
import com.alibaba.csp.sentinel.util.StringUtil; | import com.alibaba.csp.sentinel.util.StringUtil; | ||||
@@ -117,29 +118,16 @@ public class MachineInfo implements Comparable<MachineInfo> { | |||||
@Override | @Override | ||||
public boolean equals(Object o) { | public boolean equals(Object o) { | ||||
if (this == o) { | |||||
return true; | |||||
} | |||||
if (!(o instanceof MachineInfo)) { | |||||
return false; | |||||
} | |||||
if (this == o) { return true; } | |||||
if (!(o instanceof MachineInfo)) { return false; } | |||||
MachineInfo that = (MachineInfo)o; | MachineInfo that = (MachineInfo)o; | ||||
if (app != null ? !app.equals(that.app) : that.app != null) { | |||||
return false; | |||||
} | |||||
if (hostname != null ? !hostname.equals(that.hostname) : that.hostname != null) { | |||||
return false; | |||||
} | |||||
return ip != null ? ip.equals(that.ip) : that.ip == null; | |||||
return Objects.equals(app, that.app) && | |||||
Objects.equals(ip, that.ip) && | |||||
Objects.equals(port, that.port); | |||||
} | } | ||||
@Override | @Override | ||||
public int hashCode() { | public int hashCode() { | ||||
int result = app != null ? app.hashCode() : 0; | |||||
result = 31 * result + (hostname != null ? hostname.hashCode() : 0); | |||||
result = 31 * result + (ip != null ? ip.hashCode() : 0); | |||||
return result; | |||||
return Objects.hash(app, ip, port); | |||||
} | } | ||||
} | } |
@@ -0,0 +1,52 @@ | |||||
package com.taobao.csp.sentinel.dashboard.discovery; | |||||
import java.util.ConcurrentModificationException; | |||||
import java.util.Set; | |||||
import org.junit.Test; | |||||
import static org.junit.Assert.*; | |||||
public class AppInfoTest { | |||||
@Test | |||||
public void testConcurrentGetMachines() throws Exception { | |||||
AppInfo appInfo = new AppInfo("testApp"); | |||||
appInfo.addMachine(genMachineInfo("hostName1", "10.18.129.91")); | |||||
appInfo.addMachine(genMachineInfo("hostName2", "10.18.129.92")); | |||||
Set<MachineInfo> machines = appInfo.getMachines(); | |||||
new Thread(() -> { | |||||
try { | |||||
for (MachineInfo m : machines) { | |||||
System.out.println(m); | |||||
try { | |||||
Thread.sleep(200); | |||||
} catch (InterruptedException e) { | |||||
} | |||||
} | |||||
} catch (ConcurrentModificationException e) { | |||||
e.printStackTrace(); | |||||
assertTrue(false); | |||||
} | |||||
}).start(); | |||||
Thread.sleep(100); | |||||
try { | |||||
appInfo.addMachine(genMachineInfo("hostName3", "10.18.129.93")); | |||||
} catch (ConcurrentModificationException e) { | |||||
e.printStackTrace(); | |||||
assertTrue(false); | |||||
} | |||||
Thread.sleep(1000); | |||||
} | |||||
private MachineInfo genMachineInfo(String hostName, String ip) { | |||||
MachineInfo machine = new MachineInfo(); | |||||
machine.setApp("testApp"); | |||||
machine.setHostname(hostName); | |||||
machine.setIp(ip); | |||||
machine.setPort(8719); | |||||
machine.setVersion(String.valueOf(System.currentTimeMillis())); | |||||
return machine; | |||||
} | |||||
} |