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

[lucene-solr] branch branch_8x updated: SOLR-13608: Sync on TrackingBackupRepo NL changes

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

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


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

commit a557f071ca405948d835c3f7b0ed8a05cfde0625
Author: Jason Gerlowski <ja...@lucidworks.com>
AuthorDate: Mon Apr 5 13:40:56 2021 -0400

    SOLR-13608: Sync on TrackingBackupRepo NL changes
    
    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 vals 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);