You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ge...@apache.org on 2021/04/06 11:53:34 UTC

[solr] branch main updated: SOLR-13608: Sync on TrackingBackupRepo NL changes (#59)

This is an automated email from the ASF dual-hosted git repository.

gerlowskija pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/main by this push:
     new 06fad78  SOLR-13608: Sync on TrackingBackupRepo NL changes (#59)
06fad78 is described below

commit 06fad780dad915ff02008c55e12624bf882672af
Author: Jason Gerlowski <ge...@apache.org>
AuthorDate: Tue Apr 6 07:53:25 2021 -0400

    SOLR-13608: Sync on TrackingBackupRepo NL changes (#59)
    
    BackupRepositoryFactory has a special case in its 'newInstance' method.
    For 'TrackingBackupRepository' instances, BRF will modify a NamedList
    representing the TBF config - adding references to the
    SolrResourceLoader and to BRF itself.
    
    In isolation this is innocuous, but becomes a problem when
    BRF.newInstance is called concurrently from different threads.  Threads
    can race to set these NamedList keys, corrupting the NamedList in the
    process.  (i.e. NamedList values ended up in 'key' indices, and vice
    versa)
    
    This commit adds synchronization to this NL initialization, to ensure
    that the NL values are only set by a single thread.
---
 .../solr/core/backup/repository/BackupRepositoryFactory.java | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepositoryFactory.java b/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepositoryFactory.java
index 6d57a90..46143ca 100644
--- a/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepositoryFactory.java
+++ b/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepositoryFactory.java
@@ -73,9 +73,15 @@ public class BackupRepositoryFactory {
             "Could not find a backup repository with name " + name);
 
     BackupRepository result = loader.newInstance(repo.className, BackupRepository.class);
-    if ("trackingBackupRepository".equals(name) && repo.initArgs.get("factory") == null) {
-      repo.initArgs.add("factory", this);
-      repo.initArgs.add("loader", loader);
+    if ("trackingBackupRepository".equals(name)) {
+      // newInstance can be called by multiple threads, synchronization prevents simultaneous multi-threaded 'adds' from
+      // corrupting the namedlist
+      synchronized (repo.initArgs) {
+        if (repo.initArgs.get("factory") == null) {
+          repo.initArgs.add("factory", this);
+          repo.initArgs.add("loader", loader);
+        }
+      }
     }
 
     result.init(repo.initArgs);