Fix SearchContext from being closed prematurely

Fixes SearchContext from being closed during initialization or immediately after processing is started Closes #5165
elastic · Feb 19, 2014 · 9538e7d · 9538e7d
1 parent 32526f8
commit 9538e7d
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 6 deletions.
diff --git a/src/main/java/org/elasticsearch/search/SearchService.java b/src/main/java/org/elasticsearch/search/SearchService.java
@@ -465,9 +465,17 @@ private SearchContext findContext(long id) throws SearchContextMissingException
 
     SearchContext createAndPutContext(ShardSearchRequest request) throws ElasticsearchException {
         SearchContext context = createContext(request);
-        activeContexts.put(context.id(), context);
-        context.indexShard().searchService().onNewContext(context);
-        return context;
+        boolean success = false;
+        try {
+            activeContexts.put(context.id(), context);
+            context.indexShard().searchService().onNewContext(context);
+            success = true;
+            return context;
+        } finally {
+            if (!success) {
+                freeContext(context);
+            }
+        }
     }
 
     SearchContext createContext(ShardSearchRequest request) throws ElasticsearchException {
@@ -838,10 +846,14 @@ class Reaper implements Runnable {
         public void run() {
             long time = threadPool.estimatedTimeInMillis();
             for (SearchContext context : activeContexts.values()) {
-                if (context.lastAccessTime() == -1) { // its being processed or timeout is disabled
+                // Use the same value for both checks since lastAccessTime can
+                // be modified by another thread between checks!
+                long lastAccessTime = context.lastAccessTime();
+                if (lastAccessTime == -1l) { // its being processed or timeout is disabled
                     continue;
                 }
-                if ((time - context.lastAccessTime() > context.keepAlive())) {
+                if ((time - lastAccessTime > context.keepAlive())) {
+                    logger.debug("freeing search context [{}], time [{}], lastAccessTime [{}], keepAlive [{}]", context.id(), time, lastAccessTime, context.keepAlive());
                     freeContext(context);
                 }
             }

diff --git a/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java b/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java
@@ -169,7 +169,7 @@ public class DefaultSearchContext extends SearchContext {
 
     private volatile long keepAlive;
 
-    private volatile long lastAccessTime;
+    private volatile long lastAccessTime = -1;
 
     private List<Releasable> clearables = null;