From 8ec7ee66a9ca266bf0d961c28f67d121e37dc389 Mon Sep 17 00:00:00 2001
From: kimchy <kimchy@gmail.com>
Date: Wed, 21 Jul 2010 16:59:58 +0300
Subject: [PATCH] Search: Sending a request that fails to parse can cause file
 leaks, closes #270.

---
 .../elasticsearch/search/SearchService.java   | 53 ++++++++++---------
 .../search/internal/SearchContext.java        | 13 +++--
 2 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/search/SearchService.java b/modules/elasticsearch/src/main/java/org/elasticsearch/search/SearchService.java
index 7ad32788555..bae2e225b22 100644
--- a/modules/elasticsearch/src/main/java/org/elasticsearch/search/SearchService.java
+++ b/modules/elasticsearch/src/main/java/org/elasticsearch/search/SearchService.java
@@ -286,38 +286,43 @@ public class SearchService extends AbstractLifecycleComponent<SearchService> {
     private SearchContext createContext(InternalSearchRequest request) throws ElasticSearchException {
         IndexService indexService = indicesService.indexServiceSafe(request.index());
         IndexShard indexShard = indexService.shardSafe(request.shardId());
-        Engine.Searcher engineSearcher = indexShard.searcher();
 
         SearchShardTarget shardTarget = new SearchShardTarget(clusterService.state().nodes().localNodeId(), request.index(), request.shardId());
 
+        Engine.Searcher engineSearcher = indexShard.searcher();
         SearchContext context = new SearchContext(idGenerator.incrementAndGet(), shardTarget, request.numberOfShards(), request.timeout(), request.types(), engineSearcher, indexService, scriptService);
 
-        context.scroll(request.scroll());
+        try {
+            context.scroll(request.scroll());
 
-        parseSource(context, request.source(), request.sourceOffset(), request.sourceLength());
-        parseSource(context, request.extraSource(), request.extraSourceOffset(), request.extraSourceLength());
+            parseSource(context, request.source(), request.sourceOffset(), request.sourceLength());
+            parseSource(context, request.extraSource(), request.extraSourceOffset(), request.extraSourceLength());
 
-        // if the from and size are still not set, default them
-        if (context.from() == -1) {
-            context.from(0);
+            // if the from and size are still not set, default them
+            if (context.from() == -1) {
+                context.from(0);
+            }
+            if (context.size() == -1) {
+                context.size(10);
+            }
+
+            // pre process
+            dfsPhase.preProcess(context);
+            queryPhase.preProcess(context);
+            fetchPhase.preProcess(context);
+
+            // compute the context keep alive
+            TimeValue keepAlive = defaultKeepAlive;
+            if (request.scroll() != null && request.scroll().keepAlive() != null) {
+                keepAlive = request.scroll().keepAlive();
+            }
+            context.keepAlive(keepAlive);
+            context.accessed(timerService.estimatedTimeInMillis());
+            context.keepAliveTimeout(timerService.newTimeout(new KeepAliveTimerTask(context), keepAlive, TimerService.ExecutionType.DEFAULT));
+        } catch (RuntimeException e) {
+            context.release();
+            throw e;
         }
-        if (context.size() == -1) {
-            context.size(10);
-        }
-
-        // pre process
-        dfsPhase.preProcess(context);
-        queryPhase.preProcess(context);
-        fetchPhase.preProcess(context);
-
-        // compute the context keep alive
-        TimeValue keepAlive = defaultKeepAlive;
-        if (request.scroll() != null && request.scroll().keepAlive() != null) {
-            keepAlive = request.scroll().keepAlive();
-        }
-        context.keepAlive(keepAlive);
-        context.accessed(timerService.estimatedTimeInMillis());
-        context.keepAliveTimeout(timerService.newTimeout(new KeepAliveTimerTask(context), keepAlive, TimerService.ExecutionType.DEFAULT));
 
         return context;
     }
diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/search/internal/SearchContext.java b/modules/elasticsearch/src/main/java/org/elasticsearch/search/internal/SearchContext.java
index adf8a84cb49..66062fd85ca 100644
--- a/modules/elasticsearch/src/main/java/org/elasticsearch/search/internal/SearchContext.java
+++ b/modules/elasticsearch/src/main/java/org/elasticsearch/search/internal/SearchContext.java
@@ -21,7 +21,6 @@ package org.elasticsearch.search.internal;
 
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.Sort;
-import org.apache.lucene.store.AlreadyClosedException;
 import org.elasticsearch.ElasticSearchException;
 import org.elasticsearch.common.lease.Releasable;
 import org.elasticsearch.common.timer.Timeout;
@@ -45,7 +44,6 @@ import org.elasticsearch.search.fetch.script.ScriptFieldsContext;
 import org.elasticsearch.search.highlight.SearchContextHighlight;
 import org.elasticsearch.search.query.QuerySearchResult;
 
-import java.io.IOException;
 import java.util.List;
 
 /**
@@ -136,15 +134,16 @@ public class SearchContext implements Releasable {
     }
 
     @Override public boolean release() throws ElasticSearchException {
+        // we should close this searcher, since its a new one we create each time, and we use the IndexReader
         try {
             searcher.close();
-        } catch (IOException e) {
-            // ignore this exception
-        } catch (AlreadyClosedException e) {
-            // ignore this as well
+        } catch (Exception e) {
+            // ignore any exception here
         }
         engineSearcher.release();
-        keepAliveTimeout.cancel();
+        if (keepAliveTimeout != null) {
+            keepAliveTimeout.cancel();
+        }
         return true;
     }