pepness commented on code in PR #6309:
URL: https://github.com/apache/netbeans/pull/6309#discussion_r1303766806
##########
platform/o.n.bootstrap/src/org/netbeans/ProxyClassPackages.java:
##########
@@ -32,46 +31,43 @@
final class ProxyClassPackages {
private ProxyClassPackages() {
}
-
+
/** A shared map of all packages known by all classloaders. Also covers
META-INF based resources.
* It contains two kinds of keys: dot-separated package names and
slash-separated
* META-INF resource names, e.g. {"org.foobar", "/services/org.foobar.Foo"}
*/
- private static final Map<String, Set<ProxyClassLoader>> packageCoverage =
new HashMap<String, Set<ProxyClassLoader>>();
+ private static final ConcurrentMap<String, Set<ProxyClassLoader>>
packageCoverage = new ConcurrentHashMap<>();
- static synchronized void addCoveredPackages(
- ProxyClassLoader loader, Iterable<String> coveredPackages
- ) {
- for (String pkg : coveredPackages) {
- Set<ProxyClassLoader> delegates =
ProxyClassPackages.packageCoverage.get(pkg);
- if (delegates == null) {
- delegates = Collections.<ProxyClassLoader>singleton(loader);
- ProxyClassPackages.packageCoverage.put(pkg, delegates);
- } else if (delegates.size() == 1) {
- delegates = new HashSet<ProxyClassLoader>(delegates);
- ProxyClassPackages.packageCoverage.put(pkg, delegates);
- delegates.add(loader);
- } else {
- delegates.add(loader);
+ static void addCoveredPackages( ProxyClassLoader loader, Iterable<String>
coveredPackages) {
+ synchronized(packageCoverage) {
+ for (String pkg : coveredPackages) {
+ Set<ProxyClassLoader> delegates =
ProxyClassPackages.packageCoverage.get(pkg);
+ if (delegates == null) {
+ delegates =
Collections.<ProxyClassLoader>singleton(loader);
+ ProxyClassPackages.packageCoverage.put(pkg, delegates);
+ } else if (delegates.size() == 1) {
+ delegates = new HashSet<ProxyClassLoader>(delegates);
+ ProxyClassPackages.packageCoverage.put(pkg, delegates);
+ delegates.add(loader);
+ } else {
+ delegates.add(loader);
+ }
}
}
}
- static synchronized void removeCoveredPakcages(
- ProxyClassLoader loader
- ) {
- for (Iterator<String> it =
ProxyClassPackages.packageCoverage.keySet().iterator(); it.hasNext();) {
- String pkg = it.next();
- Set<ProxyClassLoader> set =
ProxyClassPackages.packageCoverage.get(pkg);
+ static void removeCoveredPakcages(ProxyClassLoader loader) {
+ packageCoverage.values().removeIf( (Set<ProxyClassLoader> set) -> {
if (set.contains(loader) && set.size() == 1) {
Review Comment:
@sdedic you are right, the current code has potential race problems.
> But I forgot the potential race between findCoveredPkg(String pkg) and
add/remove operation. It is present even in the current code !!
To prevent this I found out that the `computeIfPresent` method
implementation from ConcurrentHashMap synchronize the field that is reading to
prevent consistency issues with the other methods that are modifying the Map
(`removeCoveredPakcages` and `addCoveredPackages`), so hopefully, there should
not be any more race or consistency issues.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
