You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/01/12 08:00:20 UTC
[GitHub] [lucene-solr] muse-dev[bot] commented on a change in pull request #1666: SOLR-14155: Load all other SolrCore plugins from packages
muse-dev[bot] commented on a change in pull request #1666:
URL: https://github.com/apache/lucene-solr/pull/1666#discussion_r555575706
##########
File path: solr/core/src/java/org/apache/solr/pkg/PackageListeningClassLoader.java
##########
@@ -61,18 +66,25 @@ public PackageListeningClassLoader(CoreContainer coreContainer,
@Override
public <T> T newInstance(String cname, Class<T> expectedType, String... subpackages) {
PluginInfo.ClassName cName = new PluginInfo.ClassName(cname);
- if(cName.pkg == null){
- return coreResourceLoader.newInstance(cname, expectedType, subpackages);
+ if(cName.pkg == null) {
+ return fallbackClassLoader.newInstance(cname, expectedType, subpackages);
} else {
- PackageLoader.Package.Version version = findPkgVersion(cName);
+ PackageLoader.Package.Version version = findPackageVersion(cName, true);
return applyResourceLoaderAware(version, version.getLoader().newInstance(cName.className, expectedType, subpackages));
}
}
- private PackageLoader.Package.Version findPkgVersion(PluginInfo.ClassName cName) {
+
+ /**
+ * This looks up for package and also listens for that package if required
+ * @param cName The class name
+ */
+ public PackageLoader.Package.Version findPackageVersion(PluginInfo.ClassName cName, boolean registerListener) {
PackageLoader.Package.Version theVersion = coreContainer.getPackageLoader().getPackage(cName.pkg).getLatest(pkgVersionSupplier.apply(cName.pkg));
- packageVersions.put(cName.pkg, theVersion.getPkgVersion());
+ if(registerListener) {
+ packageVersions.put(cName.pkg, theVersion.getPkgVersion());
Review comment:
*NULL_DEREFERENCE:* object `theVersion` last assigned on line 84 could be null and is dereferenced at line 86.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -927,7 +929,7 @@ private UpdateHandler createUpdateHandler(String className, UpdateHandler update
public SolrCore(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet) {
this(coreContainer, cd, configSet, null,
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `SolrCore(...)` indirectly mutates container `util.ObjectReleaseTracker.OBJECTS` via call to `Map.put(...)` outside of synchronization.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -1443,8 +1445,8 @@ public StatsCache createStatsCache() {
final StatsCache cache;
PluginInfo pluginInfo = solrConfig.getPluginInfo(StatsCache.class.getName());
if (pluginInfo != null && pluginInfo.className != null && pluginInfo.className.length() > 0) {
- cache = createInitInstance(pluginInfo, StatsCache.class, null,
- LocalStatsCache.class.getName());
+ cache = resourceLoader.newInstance( pluginInfo, StatsCache.class, true);
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `SolrCore.createStatsCache()` indirectly mutates container `core.SolrResourceLoader.classNameCache` via call to `Map.put(...)` outside of synchronization.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -2134,12 +2136,12 @@ public IndexFingerprint getIndexFingerprint(SolrIndexSearcher searcher, LeafRead
newReader = currentReader;
}
- // for now, turn off caches if this is for a realtime reader
+ // for now, turn off caches if this is for a realtime reader
// (caches take a little while to instantiate)
final boolean useCaches = !realtime;
final String newName = realtime ? "realtime" : "main";
tmp = new SolrIndexSearcher(this, newIndexDir, getLatestSchema(), newName,
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `SolrCore.openNewSearcher(...)` indirectly reads with synchronization from container `core.SolrResourceLoader.classNameCache` via call to `Map.get(...)`. Potentially races with unsynchronized write in method `SolrCore.initIndex(...)`.
Reporting because this access may occur on a background thread.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
##########
@@ -834,6 +847,47 @@ public void close() throws IOException {
public List<SolrInfoBean> getInfoMBeans() {
return Collections.unmodifiableList(infoMBeans);
}
+ /**
+ * Load a class using an appropriate {@link SolrResourceLoader} depending of the package on that class
+ * @param registerCoreReloadListener register a listener for the package and reload the core if the package is changed.
+ * Use this sparingly. This will result in core reloads across all the cores in
+ * all collections using this configset
+ */
+ public <T> Class<? extends T> findClass( PluginInfo info, Class<T> type, boolean registerCoreReloadListener) {
+ if(info.cName.pkg == null) return findClass(info.className, type);
+ return _classLookup(info,
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `SolrResourceLoader.findClass(...)` indirectly reads without synchronization from `this.coreReloadingClassLoader`. Potentially races with write in method `SolrResourceLoader.inform(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
##########
@@ -654,6 +657,16 @@ public void inform(SolrCore core) {
this.config = core.getSolrConfig();
this.coreId = core.uniqueId;
this.coreContainer = core.getCoreContainer();
+ SolrCore.Provider coreProvider = core.coreProvider;
+
+ this.coreReloadingClassLoader = new PackageListeningClassLoader(core.getCoreContainer(),
+ this, s -> config.maxPackageVersion(s), null){
+ @Override
+ protected void doReloadAction(Ctx ctx) {
+ coreProvider.reload();
+ }
+ };
+ core.getPackageListeners().addListener(coreReloadingClassLoader, true);
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `SolrResourceLoader.inform(...)` reads without synchronization from `this.coreReloadingClassLoader`. Potentially races with write in method `SolrResourceLoader.inform(...)`.
Reporting because this access may occur on a background thread.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
##########
@@ -654,6 +657,16 @@ public void inform(SolrCore core) {
this.config = core.getSolrConfig();
this.coreId = core.uniqueId;
this.coreContainer = core.getCoreContainer();
+ SolrCore.Provider coreProvider = core.coreProvider;
+
+ this.coreReloadingClassLoader = new PackageListeningClassLoader(core.getCoreContainer(),
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `SolrResourceLoader.inform(...)` writes to field `this.coreReloadingClassLoader` outside of synchronization.
Reporting because this access may occur on a background thread.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
##########
@@ -834,6 +847,47 @@ public void close() throws IOException {
public List<SolrInfoBean> getInfoMBeans() {
return Collections.unmodifiableList(infoMBeans);
}
+ /**
+ * Load a class using an appropriate {@link SolrResourceLoader} depending of the package on that class
+ * @param registerCoreReloadListener register a listener for the package and reload the core if the package is changed.
+ * Use this sparingly. This will result in core reloads across all the cores in
+ * all collections using this configset
+ */
+ public <T> Class<? extends T> findClass( PluginInfo info, Class<T> type, boolean registerCoreReloadListener) {
+ if(info.cName.pkg == null) return findClass(info.className, type);
+ return _classLookup(info,
+ (Function<PackageLoader.Package.Version, Class<? extends T>>) ver -> ver.getLoader().findClass(info.cName.className, type), registerCoreReloadListener);
+
+ }
+
+
+ private <T> T _classLookup(PluginInfo info, Function<PackageLoader.Package.Version, T> fun, boolean registerCoreReloadListener ) {
+ PluginInfo.ClassName cName = info.cName;
+ if (registerCoreReloadListener) {
+ if (coreReloadingClassLoader == null) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Core not set");
+ }
+ return fun.apply(coreReloadingClassLoader.findPackageVersion(cName, true));
+ } else {
+ return fun.apply(coreReloadingClassLoader.findPackageVersion(cName, false));
+ }
+ }
+
+ /**
+ *Create a n instance of a class using an appropriate {@link SolrResourceLoader} depending on the package of that class
+ * @param registerCoreReloadListener register a listener for the package and reload the core if the package is changed.
+ * Use this sparingly. This will result in core reloads across all the cores in
+ * all collections using this configset
+ */
+ public <T> T newInstance(PluginInfo info, Class<T> type, boolean registerCoreReloadListener) {
+ if(info.cName.pkg == null) {
+ return newInstance(info.cName.className == null?
+ type.getName():
+ info.cName.className ,
+ type);
+ }
+ return _classLookup( info, version -> version.getLoader().newInstance(info.cName.className, type), registerCoreReloadListener);
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `SolrResourceLoader.newInstance(...)` indirectly reads without synchronization from `this.coreReloadingClassLoader`. Potentially races with write in method `SolrResourceLoader.inform(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org