Search: Sending a request that fails to parse can cause file leaks, c…

…loses #270.
elastic · Jul 21, 2010 · 8ec7ee6 · 8ec7ee6
1 parent 0cb97e4
commit 8ec7ee6
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 29 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
@@ -286,38 +286,43 @@ private SearchContext findContext(long id) throws SearchContextMissingException
     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 (context.size() == -1) {
-            context.size(10);
-        }
+            // 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);
+            // 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();
+            // 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;
         }
-        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
@@ -21,7 +21,6 @@
 
 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.highlight.SearchContextHighlight;
 import org.elasticsearch.search.query.QuerySearchResult;
 
-import java.io.IOException;
 import java.util.List;
 
 /**
@@ -136,15 +134,16 @@ public SearchContext(long id, SearchShardTarget shardTarget, int numberOfShards,
     }
 
     @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;
     }