Skip to content

Commit 72ad520

Browse files
committed
JAVA-610: Make DBCursor finalizer optional and disable it in all cases where there is no cursor maintained on the server
1 parent f3e61e4 commit 72ad520

File tree

5 files changed

+120
-18
lines changed

5 files changed

+120
-18
lines changed

src/main/com/mongodb/DBApiLayer.java

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,9 @@ class Result implements Iterator<DBObject> {
378378
_host = res._host;
379379
_decoder = decoder;
380380
init( res );
381+
// Only enable finalizer if cursor finalization is enabled and there is actually a cursor that needs killing
382+
_optionalFinalizer = _mongo.getMongoOptions().isCursorFinalizerEnabled() && res.cursor() != 0 ?
383+
new OptionalFinalizer() : null;
381384
}
382385

383386
private void init( Response res ){
@@ -471,18 +474,6 @@ public String toString(){
471474
return "DBCursor";
472475
}
473476

474-
protected void finalize() throws Throwable {
475-
if (_curResult != null) {
476-
long curId = _curResult.cursor();
477-
_curResult = null;
478-
_cur = null;
479-
if (curId != 0) {
480-
_deadCursorIds.add(new DeadCursor(curId, _host));
481-
}
482-
}
483-
super.finalize();
484-
}
485-
486477
public long totalBytes(){
487478
return _totalBytes;
488479
}
@@ -533,6 +524,10 @@ public ServerAddress getServerAddress() {
533524
return _host;
534525
}
535526

527+
boolean hasFinalizer() {
528+
return _optionalFinalizer != null;
529+
}
530+
536531
Response _curResult;
537532
Iterator<DBObject> _cur;
538533
int _batchSize;
@@ -547,6 +542,23 @@ public ServerAddress getServerAddress() {
547542
private List<Integer> _sizes = new ArrayList<Integer>();
548543
private int _numFetched = 0;
549544

545+
// This allows us to easily enable/disable finalizer for cleaning up un-closed cursors
546+
private final OptionalFinalizer _optionalFinalizer;
547+
548+
private class OptionalFinalizer {
549+
@Override
550+
protected void finalize() {
551+
if (_curResult != null) {
552+
long curId = _curResult.cursor();
553+
_curResult = null;
554+
_cur = null;
555+
if (curId != 0) {
556+
_deadCursorIds.add(new DeadCursor(curId, _host));
557+
}
558+
}
559+
}
560+
}
561+
550562
} // class Result
551563

552564
static class DeadCursor {

src/main/com/mongodb/DBCursor.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,13 @@ public String toString() {
693693
return sb.toString();
694694
}
695695

696+
boolean hasFinalizer() {
697+
if (_it == null || ! (_it instanceof Result)) {
698+
return false;
699+
}
700+
return ((Result) _it).hasFinalizer();
701+
}
702+
696703
// ---- query setup ----
697704
private final DBCollection _collection;
698705
private final DBObject _query;

src/main/com/mongodb/MongoOptions.java

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ public void reset(){
5050
dbEncoderFactory = DefaultDBEncoder.FACTORY;
5151
socketFactory = SocketFactory.getDefault();
5252
description = null;
53+
cursorFinalizerEnabled = true;
5354
}
5455

5556
public MongoOptions copy() {
@@ -73,6 +74,7 @@ public MongoOptions copy() {
7374
m.dbEncoderFactory = dbEncoderFactory;
7475
m.socketFactory = socketFactory;
7576
m.description = description;
77+
m.cursorFinalizerEnabled = cursorFinalizerEnabled;
7678
return m;
7779
}
7880

@@ -226,6 +228,16 @@ else if (safe)
226228
*/
227229
public SocketFactory socketFactory;
228230

231+
/**
232+
* Sets whether there is a a finalize method created that cleans up instances of DBCursor that the client
233+
* does not close. If you are careful to always call the close method of DBCursor, then this can safely be set to false.
234+
* @see com.mongodb.DBCursor#close().
235+
* Default is true.
236+
*/
237+
public boolean cursorFinalizerEnabled;
238+
239+
240+
229241
public String toString(){
230242
StringBuilder buf = new StringBuilder();
231243
buf.append( "description=" ).append( description ).append( ", " );
@@ -245,7 +257,8 @@ public String toString(){
245257
buf.append( "w=" ).append( w ).append( ", " );
246258
buf.append( "wtimeout=" ).append( wtimeout ).append( ", " );
247259
buf.append( "fsync=" ).append( fsync ).append( ", " );
248-
buf.append( "j=" ).append( j );
260+
buf.append( "j=" ).append(j).append( ", " );
261+
buf.append( "cursorFinalizerEnabled=").append( cursorFinalizerEnabled);
249262

250263
return buf.toString();
251264
}
@@ -538,4 +551,21 @@ public ReadPreference getReadPreference() {
538551
public void setReadPreference(ReadPreference readPreference) {
539552
this.readPreference = readPreference;
540553
}
554+
555+
556+
/**
557+
*
558+
* @return whether DBCursor finalizer is enabled
559+
*/
560+
public boolean isCursorFinalizerEnabled() {
561+
return cursorFinalizerEnabled;
562+
}
563+
564+
/**
565+
*
566+
* @param cursorFinalizerEnabled whether cursor finalizer is enabled
567+
*/
568+
public void setCursorFinalizerEnabled(final boolean cursorFinalizerEnabled) {
569+
this.cursorFinalizerEnabled = cursorFinalizerEnabled;
570+
}
541571
}

src/test/com/mongodb/DBCursorTest.java

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,19 @@
1616

1717
package com.mongodb;
1818

19-
import java.io.IOException;
20-
import java.util.Iterator;
21-
22-
import java.util.concurrent.*;
19+
import com.mongodb.util.TestCase;
2320
import org.testng.annotations.Test;
2421

25-
import com.mongodb.util.TestCase;
22+
import java.io.IOException;
23+
import java.net.UnknownHostException;
24+
import java.util.Iterator;
25+
import java.util.concurrent.Callable;
26+
import java.util.concurrent.ExecutionException;
27+
import java.util.concurrent.ExecutorService;
28+
import java.util.concurrent.Executors;
29+
import java.util.concurrent.Future;
30+
import java.util.concurrent.TimeUnit;
31+
import java.util.concurrent.TimeoutException;
2632

2733
public class DBCursorTest extends TestCase {
2834

@@ -456,6 +462,49 @@ public void testSort(){
456462
curmax = val;
457463
}
458464
}
465+
466+
@Test
467+
public void testHasFinalizer() throws UnknownHostException {
468+
DBCollection c = _db.getCollection( "HasFinalizerTest" );
469+
c.drop();
470+
471+
for ( int i=0; i<1000; i++ )
472+
c.save( new BasicDBObject("_id", i), WriteConcern.SAFE);
473+
474+
// finalizer is on by default so after calling hasNext should report that it has one
475+
DBCursor cursor = c.find();
476+
assertFalse(cursor.hasFinalizer());
477+
cursor.hasNext();
478+
assertTrue(cursor.hasFinalizer());
479+
cursor.close();
480+
481+
// no finalizer if there is no cursor, as there should not be for a query with only one result
482+
cursor = c.find(new BasicDBObject("_id", 1));
483+
cursor.hasNext();
484+
assertFalse(cursor.hasFinalizer());
485+
cursor.close();
486+
487+
// no finalizer if there is no cursor, as there should not be for a query with negative batch size
488+
cursor = c.find();
489+
cursor.batchSize(-1);
490+
cursor.hasNext();
491+
assertFalse(cursor.hasFinalizer());
492+
cursor.close();
493+
494+
// finally, no finalizer if disabled in mongo options
495+
MongoOptions mongoOptions = new MongoOptions();
496+
mongoOptions.cursorFinalizerEnabled = false;
497+
Mongo m = new Mongo("127.0.0.1", mongoOptions);
498+
try {
499+
c = m.getDB(cleanupDB).getCollection("HasFinalizerTest");
500+
cursor = c.find();
501+
cursor.hasNext();
502+
assertFalse(cursor.hasFinalizer());
503+
cursor.close();
504+
} finally {
505+
m.close();
506+
}
507+
}
459508

460509
final DB _db;
461510

src/test/com/mongodb/MongoOptionsTest.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ public void testCopy() throws Exception {
5050
options.socketFactory = null;
5151
options.description = "cool";
5252
options.readPreference = ReadPreference.secondary();
53+
options.cursorFinalizerEnabled = true;
5354

5455
final MongoOptions copy = options.copy();
5556
assertEquals(options.connectionsPerHost, copy.connectionsPerHost);
@@ -71,6 +72,7 @@ public void testCopy() throws Exception {
7172
assertEquals(options.socketFactory, copy.socketFactory);
7273
assertEquals(options.description, copy.description);
7374
assertEquals(options.readPreference, copy.readPreference);
75+
assertEquals(options.cursorFinalizerEnabled, copy.cursorFinalizerEnabled);
7476
}
7577

7678
@Test
@@ -96,6 +98,7 @@ public void testGetterSetters() throws Exception {
9698
options.setSocketFactory(null);
9799
options.setDescription("very cool");
98100
options.setReadPreference(ReadPreference.secondary());
101+
options.setCursorFinalizerEnabled(true);
99102

100103
assertEquals(options.getConnectionsPerHost(), 100);
101104
assertEquals(options.getThreadsAllowedToBlockForConnectionMultiplier(), 101);
@@ -115,6 +118,7 @@ public void testGetterSetters() throws Exception {
115118
assertEquals(options.getSocketFactory(), null);
116119
assertEquals(options.getDescription(), "very cool");
117120
assertEquals(options.getReadPreference(), ReadPreference.secondary());
121+
assertEquals(options.isCursorFinalizerEnabled(), true);
118122
}
119123
}
120124

0 commit comments

Comments
 (0)