From 7f14674ff5d49d27c458ce3e7fa455457add6191 Mon Sep 17 00:00:00 2001
From: Boaz Leskes <b.leskes@gmail.com>
Date: Sun, 26 Oct 2014 19:03:33 +0100
Subject: [PATCH] Recovery: RecoveriesCollection.findRecoveryByShard should
 call recoveryStatus.tryIncRef before accessing fields

The function iterates over a snapshot of on going recoveries and thus may access RecoveryStatus objects that are already finished.

Closes #8231
---
 .../recovery/RecoveriesCollection.java        | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/main/java/org/elasticsearch/indices/recovery/RecoveriesCollection.java b/src/main/java/org/elasticsearch/indices/recovery/RecoveriesCollection.java
index 80a7b598369..ff817c4551c 100644
--- a/src/main/java/org/elasticsearch/indices/recovery/RecoveriesCollection.java
+++ b/src/main/java/org/elasticsearch/indices/recovery/RecoveriesCollection.java
@@ -103,8 +103,8 @@ public class RecoveriesCollection {
     /**
      * fail the recovery with the given id (if found) and remove it from the recovery collection
      *
-     * @param id id of the recovery to fail
-     * @param e  exception with reason for the failure
+     * @param id               id of the recovery to fail
+     * @param e                exception with reason for the failure
      * @param sendShardFailure true a shard failed message should be sent to the master
      */
     public void failRecovery(long id, RecoveryFailedException e, boolean sendShardFailure) {
@@ -130,11 +130,16 @@ public class RecoveriesCollection {
     @Nullable
     public StatusRef findRecoveryByShard(IndexShard indexShard) {
         for (RecoveryStatus recoveryStatus : onGoingRecoveries.values()) {
-            if (recoveryStatus.indexShard() == indexShard) {
-                if (recoveryStatus.tryIncRef()) {
-                    return new StatusRef(recoveryStatus);
-                } else {
-                    return null;
+            // check if the recovery has already finished and if not protect
+            // against it being closed on us while we check
+            if (recoveryStatus.tryIncRef()) {
+                try {
+                    if (recoveryStatus.indexShard() == indexShard) {
+                        recoveryStatus.incRef();
+                        return new StatusRef(recoveryStatus);
+                    }
+                } finally {
+                    recoveryStatus.decRef();
                 }
             }
         }