From 5b56112dbe66cb94c33c6f6268076018000e6243 Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Tue, 28 Jun 2011 11:55:34 +0000
Subject: [PATCH] Fix OPENDJ-219 - Replication server and draft changelog DB code may attempt to reference closed DB
---
opends/src/server/org/opends/server/replication/server/DraftCNDbIterator.java | 10 --
opends/src/server/org/opends/server/replication/server/ReplicationDB.java | 92 +++++++++++++++++++++-
opends/src/server/org/opends/server/replication/server/ReplicationServerDomain.java | 20 +++--
opends/src/server/org/opends/server/replication/server/DraftCNDB.java | 107 ++++++++++++++++++++++++++
4 files changed, 204 insertions(+), 25 deletions(-)
diff --git a/opends/src/server/org/opends/server/replication/server/DraftCNDB.java b/opends/src/server/org/opends/server/replication/server/DraftCNDB.java
index c9f23e5..aaae7b8 100644
--- a/opends/src/server/org/opends/server/replication/server/DraftCNDB.java
+++ b/opends/src/server/org/opends/server/replication/server/DraftCNDB.java
@@ -103,6 +103,12 @@
dbCloseLock.readLock().lock();
try
{
+ // If the DB has been closed then return immediately.
+ if (isDBClosed())
+ {
+ return;
+ }
+
txn = dbenv.beginTransaction();
db.put(txn, key, data);
txn.commit(Durability.COMMIT_WRITE_NO_SYNC);
@@ -141,6 +147,7 @@
try
{
db.close();
+ db = null;
}
finally
{
@@ -221,6 +228,12 @@
Cursor cursor = null;
try
{
+ // If the DB has been closed then return immediately.
+ if (isDBClosed())
+ {
+ return 0;
+ }
+
cursor = db.openCursor(null, null);
DatabaseEntry key = new DatabaseEntry();
DatabaseEntry entry = new DatabaseEntry();
@@ -252,14 +265,25 @@
*/
public long count()
{
+ dbCloseLock.readLock().lock();
try
{
+ // If the DB has been closed then return immediately.
+ if (isDBClosed())
+ {
+ return 0L;
+ }
+
return db.count();
}
- catch(Exception e)
+ catch (Exception e)
{
TRACER.debugCaught(DebugLogLevel.ERROR, e);
}
+ finally
+ {
+ dbCloseLock.readLock().unlock();
+ }
return 0L;
}
@@ -275,6 +299,12 @@
Cursor cursor = null;
try
{
+ // If the DB has been closed then return immediately.
+ if (isDBClosed())
+ {
+ return 0;
+ }
+
cursor = db.openCursor(null, null);
DatabaseEntry key = new DatabaseEntry();
DatabaseEntry entry = new DatabaseEntry();
@@ -351,6 +381,15 @@
try
{
+ // If the DB has been closed then create empty cursor.
+ if (isDBClosed())
+ {
+ isClosed = true;
+ txn = null;
+ cursor = null;
+ return;
+ }
+
localCursor = db.openCursor(localTxn, null);
if (startingDraftCN >= 0)
{
@@ -410,6 +449,15 @@
dbCloseLock.readLock().lock();
try
{
+ // If the DB has been closed then create empty cursor.
+ if (isDBClosed())
+ {
+ isClosed = true;
+ txn = null;
+ cursor = null;
+ return;
+ }
+
// Create the transaction that will protect whatever done with this
// write cursor.
localTxn = dbenv.beginTransaction();
@@ -516,6 +564,11 @@
*/
public String currentValue()
{
+ if (isClosed)
+ {
+ return null;
+ }
+
try
{
if (seqnumData != null)
@@ -536,6 +589,11 @@
*/
public String currentServiceID()
{
+ if (isClosed)
+ {
+ return null;
+ }
+
try
{
if (seqnumData != null)
@@ -547,6 +605,7 @@
{
TRACER.debugCaught(DebugLogLevel.ERROR, e);
}
+
return null;
}
@@ -558,16 +617,22 @@
*/
public int currentKey()
{
- try
+ if (isClosed)
+ {
+ return -1;
+ }
+
+ try
{
String str = decodeUTF8(key.getData());
int draftCN = new Integer(str);
return draftCN;
}
- catch(Exception e)
+ catch (Exception e)
{
TRACER.debugCaught(DebugLogLevel.ERROR, e);
}
+
return -1;
}
@@ -577,6 +642,11 @@
*/
public ChangeNumber currentChangeNumber()
{
+ if (isClosed)
+ {
+ return null;
+ }
+
try
{
if (seqnumData != null)
@@ -598,6 +668,11 @@
*/
public boolean next() throws DatabaseException
{
+ if (isClosed)
+ {
+ return false;
+ }
+
OperationStatus status = cursor.getNext(key, entry, LockMode.DEFAULT);
if (status != OperationStatus.SUCCESS)
{
@@ -621,6 +696,11 @@
*/
public void delete() throws DatabaseException
{
+ if (isClosed)
+ {
+ throw new IllegalStateException("DraftCNDB already closed");
+ }
+
cursor.delete();
}
@@ -631,6 +711,11 @@
*/
public DatabaseEntry getKey()
{
+ if (isClosed)
+ {
+ return null;
+ }
+
return key;
}
}
@@ -647,10 +732,17 @@
dbCloseLock.writeLock().lock();
try
{
+ // If the DB has been closed then return immediately.
+ if (isDBClosed())
+ {
+ return;
+ }
+
String dbName = db.getDatabaseName();
// Closing is requested by the Berkeley DB before truncate
db.close();
+ db = null; // In case there's a failure between here and recreation.
// Clears the changes
dbenv.clearDb(dbName);
@@ -672,4 +764,13 @@
dbCloseLock.writeLock().unlock();
}
}
+
+
+
+ //Returns {@code true} if the DB is closed. This method assumes that either
+ // the db read/write lock has been taken.
+ private boolean isDBClosed()
+ {
+ return db == null;
+ }
}
diff --git a/opends/src/server/org/opends/server/replication/server/DraftCNDbIterator.java b/opends/src/server/org/opends/server/replication/server/DraftCNDbIterator.java
index 2c6e2fd..bd11e5f 100644
--- a/opends/src/server/org/opends/server/replication/server/DraftCNDbIterator.java
+++ b/opends/src/server/org/opends/server/replication/server/DraftCNDbIterator.java
@@ -122,15 +122,7 @@
public boolean next()
throws Exception, DatabaseException
{
- boolean hasNext = draftCNDbCursor.next();
- if (hasNext)
- {
- return true;
- }
- else
- {
- return false;
- }
+ return draftCNDbCursor.next();
}
/**
diff --git a/opends/src/server/org/opends/server/replication/server/ReplicationDB.java b/opends/src/server/org/opends/server/replication/server/ReplicationDB.java
index 411c776..9487ede 100644
--- a/opends/src/server/org/opends/server/replication/server/ReplicationDB.java
+++ b/opends/src/server/org/opends/server/replication/server/ReplicationDB.java
@@ -162,6 +162,8 @@
}
}
+
+
/**
* add a list of changes to the underlying db.
*
@@ -172,8 +174,15 @@
try
{
dbCloseLock.readLock().lock();
+
try
{
+ // If the DB has been closed then return immediately.
+ if (isDBClosed())
+ {
+ return;
+ }
+
for (UpdateMsg change : changes)
{
DatabaseEntry key = new ReplicationKey(
@@ -226,6 +235,7 @@
try
{
db.close();
+ db = null;
}
finally
{
@@ -313,6 +323,12 @@
dbCloseLock.readLock().lock();
try
{
+ // If the DB has been closed then return immediately.
+ if (isDBClosed())
+ {
+ return null;
+ }
+
DatabaseEntry key = new DatabaseEntry();
DatabaseEntry data = new DatabaseEntry();
@@ -375,6 +391,12 @@
dbCloseLock.readLock().lock();
try
{
+ // If the DB has been closed then return immediately.
+ if (isDBClosed())
+ {
+ return null;
+ }
+
DatabaseEntry key = new DatabaseEntry();
DatabaseEntry data = new DatabaseEntry();
@@ -450,6 +472,12 @@
dbCloseLock.readLock().lock();
try
{
+ // If the DB has been closed then return immediately.
+ if (isDBClosed())
+ {
+ return null;
+ }
+
cursor = db.openCursor(null, null);
if (cursor.getSearchKeyRange(key, data, LockMode.DEFAULT)
== OperationStatus.SUCCESS)
@@ -580,6 +608,14 @@
Cursor localCursor = null;
try
{
+ // If the DB has been closed then create empty cursor.
+ if (isDBClosed())
+ {
+ isClosed = true;
+ cursor = null;
+ return;
+ }
+
localCursor = db.openCursor(txn, null);
if (startingChangeNumber != null)
{
@@ -630,6 +666,15 @@
Cursor localCursor = null;
try
{
+ // If the DB has been closed then create empty cursor.
+ if (isDBClosed())
+ {
+ isClosed = true;
+ txn = null;
+ cursor = null;
+ return;
+ }
+
// Create the transaction that will protect whatever done with this
// write cursor.
localTxn = dbenv.beginTransaction();
@@ -727,6 +772,11 @@
*/
public ChangeNumber nextChangeNumber() throws DatabaseException
{
+ if (isClosed)
+ {
+ return null;
+ }
+
OperationStatus status = cursor.getNext(key, data, LockMode.DEFAULT);
if (status != OperationStatus.SUCCESS)
@@ -744,6 +794,11 @@
*/
public UpdateMsg next()
{
+ if (isClosed)
+ {
+ return null;
+ }
+
UpdateMsg currentChange = null;
while (currentChange == null)
{
@@ -794,6 +849,11 @@
*/
public void delete() throws DatabaseException
{
+ if (isClosed)
+ {
+ throw new IllegalStateException("ReplServerDBCursor already closed");
+ }
+
cursor.delete();
}
} // ReplServerDBCursor
@@ -810,6 +870,12 @@
dbCloseLock.writeLock().lock();
try
{
+ // If the DB has been closed then return immediately.
+ if (isDBClosed())
+ {
+ return;
+ }
+
String dbName = db.getDatabaseName();
// Clears the reference to this serverID
@@ -817,6 +883,7 @@
// Closing is requested by the Berkeley DB before truncate
db.close();
+ db = null; // In case there's a failure between here and recreation.
// Clears the changes
dbenv.clearDb(dbName);
@@ -843,7 +910,7 @@
* @param start The lower limit of the count.
* @param stop The higher limit of the count.
* @return The number of changes between provided start and stop changeNumber.
- * Returns -1 when an error occurs.
+ * Returns 0 when an error occurs.
*/
public int count(ChangeNumber start, ChangeNumber stop)
{
@@ -853,11 +920,20 @@
int distBackToCounterRecord2 = 0;
int count=0;
OperationStatus status;
+
try
{
+ dbCloseLock.readLock().lock();
+
Cursor cursor = null;
try
{
+ // If the DB has been closed then return immediately.
+ if (isDBClosed())
+ {
+ return 0;
+ }
+
ChangeNumber cn ;
if ((start==null)&&(stop==null))
@@ -977,10 +1053,7 @@
}
finally
{
- if (cursor != null)
- {
- cursor.close();
- }
+ closeLockedCursor(cursor);
}
}
catch (DatabaseException e)
@@ -1032,4 +1105,13 @@
this.counterWindowSize = size;
}
+
+
+ // Returns {@code true} if the DB is closed. This method assumes that either
+ // the db read/write lock has been taken.
+ private boolean isDBClosed()
+ {
+ return db == null;
+ }
+
}
diff --git a/opends/src/server/org/opends/server/replication/server/ReplicationServerDomain.java b/opends/src/server/org/opends/server/replication/server/ReplicationServerDomain.java
index 64cc575..e4ddeb3 100644
--- a/opends/src/server/org/opends/server/replication/server/ReplicationServerDomain.java
+++ b/opends/src/server/org/opends/server/replication/server/ReplicationServerDomain.java
@@ -1424,19 +1424,23 @@
if (handler == null)
return null;
+ ReplicationIterator it;
try
{
- ReplicationIterator it = handler.generateIterator(changeNumber);
- if (it.next()==false)
- {
- it.releaseCursor();
- throw new Exception("no new change");
- }
- return it;
- } catch (Exception e)
+ it = handler.generateIterator(changeNumber);
+ }
+ catch (Exception e)
{
return null;
}
+
+ if (it.next() == false)
+ {
+ it.releaseCursor();
+ return null;
+ }
+
+ return it;
}
/**
--
Gitblit v1.10.0