sdedic commented on code in PR #6309:
URL: https://github.com/apache/netbeans/pull/6309#discussion_r1298132594
##########
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:
Hm ... it does. But I forgot the potential race between
`findCoveredPkg(String pkg)` and `add/remove` operation. It is present even in
the current code !!
Imagine (even with the current code) `findCoveredPkg` returning a live Set
from the internal strucutre, and then, in parallel to the reading thread, `add`
adds a new package to the set.
It's OK for transition between 0-1-many loaders, but unsafe if there are 2+
loaders and the set is being changed. A copy-on-write in add/remove seems as
the best solution: gets greatly outnumber the mutations.
--
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
