From bf62fe2d81a22e50065a0c682a4ffb2a1308122f Mon Sep 17 00:00:00 2001
From: neil_a_wilson <neil_a_wilson@localhost>
Date: Fri, 02 Feb 2007 20:35:01 +0000
Subject: [PATCH] Update the code involving change notification listeners to ensure that they are invoked just before the response is sent to the client rather than just after the response. This can help avoid race conditions in which the server needs to use a change notification listener to perform some additional processing for a given operation, and the client sends a second request that depends on this processing immediately after receiving a "success" response for the operation that triggered the change notification.
---
opends/src/server/org/opends/server/core/AddOperation.java | 33 ++++---
opends/src/server/org/opends/server/core/ModifyDNOperation.java | 33 ++++---
opends/src/server/org/opends/server/core/ModifyOperation.java | 33 ++++---
opends/tests/unit-tests-testng/src/server/org/opends/server/core/TestChangeNotificationListener.java | 14 ++-
opends/src/server/org/opends/server/core/DeleteOperation.java | 35 +++++---
opends/src/server/org/opends/server/core/GroupManager.java | 12 ++-
opends/src/server/org/opends/server/api/ChangeNotificationListener.java | 42 ++++++----
7 files changed, 119 insertions(+), 83 deletions(-)
diff --git a/opends/src/server/org/opends/server/api/ChangeNotificationListener.java b/opends/src/server/org/opends/server/api/ChangeNotificationListener.java
index 86a07e7..554e043 100644
--- a/opends/src/server/org/opends/server/api/ChangeNotificationListener.java
+++ b/opends/src/server/org/opends/server/api/ChangeNotificationListener.java
@@ -22,17 +22,18 @@
* CDDL HEADER END
*
*
- * Portions Copyright 2006 Sun Microsystems, Inc.
+ * Portions Copyright 2006-2007 Sun Microsystems, Inc.
*/
package org.opends.server.api;
-import org.opends.server.core.AddOperation;
-import org.opends.server.core.DeleteOperation;
-import org.opends.server.core.ModifyOperation;
-import org.opends.server.core.ModifyDNOperation;
import org.opends.server.types.Entry;
+import org.opends.server.types.operation.PostResponseAddOperation;
+import org.opends.server.types.operation.PostResponseDeleteOperation;
+import org.opends.server.types.operation.PostResponseModifyOperation;
+import org.opends.server.types.operation.
+ PostResponseModifyDNOperation;
@@ -46,11 +47,15 @@
* be invoked for successful operations.
* <BR><BR>
* Each change notification listener will be notified whenever an
- * update is made in the server (just after the response has been sent
- * to the client), so the listener should use a very efficient
- * mechanism for determining whether or not any action is required for
- * the associated operation and quickly return for cases in which the
- * update is not applicable.
+ * update is made in the server (just before the response is sent to
+ * the client), so the listener should use a very efficient mechanism
+ * for determining whether or not any action is required for the
+ * associated operation and quickly return for cases in which the
+ * update is not applicable. Also note that even though the listener
+ * will be invoked before the response is sent to the client, it may
+ * not alter that response in any way and therefore the listener will
+ * be given what is essentially a read-only view of the associated
+ * operation.
* <BR><BR>
* Note that while this interface can be used by clients to be
* notified of changes to the configuration data just as easily as it
@@ -73,8 +78,9 @@
* server.
* @param entry The entry that was added to the server.
*/
- public void handleAddOperation(AddOperation addOperation,
- Entry entry);
+ public void handleAddOperation(
+ PostResponseAddOperation addOperation,
+ Entry entry);
@@ -87,8 +93,9 @@
* @param entry The entry that was removed from the
* server.
*/
- public void handleDeleteOperation(DeleteOperation deleteOperation,
- Entry entry);
+ public void handleDeleteOperation(
+ PostResponseDeleteOperation deleteOperation,
+ Entry entry);
@@ -101,8 +108,9 @@
* @param oldEntry The entry before it was updated.
* @param newEntry The entry after it was updated.
*/
- public void handleModifyOperation(ModifyOperation modifyOperation,
- Entry oldEntry, Entry newEntry);
+ public void handleModifyOperation(
+ PostResponseModifyOperation modifyOperation,
+ Entry oldEntry, Entry newEntry);
@@ -116,7 +124,7 @@
* @param newEntry The entry after it was updated.
*/
public void handleModifyDNOperation(
- ModifyDNOperation modifyDNOperation,
+ PostResponseModifyDNOperation modifyDNOperation,
Entry oldEntry, Entry newEntry);
}
diff --git a/opends/src/server/org/opends/server/core/AddOperation.java b/opends/src/server/org/opends/server/core/AddOperation.java
index f0f9f86..2e4b8bd 100644
--- a/opends/src/server/org/opends/server/core/AddOperation.java
+++ b/opends/src/server/org/opends/server/core/AddOperation.java
@@ -2154,20 +2154,8 @@
}
- // Stop the processing timer.
- processingStopTime = System.currentTimeMillis();
-
-
- // Send the add response to the client.
- getClientConnection().sendResponse(this);
-
-
- // Log the add response.
- logAddResponse(this);
-
-
- // Notify any change listeners and/or persistent searches that might be
- // registered with the server.
+ // Notify any change notification listeners that might be registered with
+ // the server.
if (getResultCode() == ResultCode.SUCCESS)
{
for (ChangeNotificationListener changeListener :
@@ -2187,7 +2175,24 @@
message, msgID);
}
}
+ }
+
+ // Stop the processing timer.
+ processingStopTime = System.currentTimeMillis();
+
+
+ // Send the add response to the client.
+ getClientConnection().sendResponse(this);
+
+
+ // Log the add response.
+ logAddResponse(this);
+
+
+ // Notify any persistent searches that might be registered with the server.
+ if (getResultCode() == ResultCode.SUCCESS)
+ {
for (PersistentSearch persistentSearch :
DirectoryServer.getPersistentSearches())
{
diff --git a/opends/src/server/org/opends/server/core/DeleteOperation.java b/opends/src/server/org/opends/server/core/DeleteOperation.java
index 93d3841..f144e2c 100644
--- a/opends/src/server/org/opends/server/core/DeleteOperation.java
+++ b/opends/src/server/org/opends/server/core/DeleteOperation.java
@@ -22,7 +22,7 @@
* CDDL HEADER END
*
*
- * Portions Copyright 2006 Sun Microsystems, Inc.
+ * Portions Copyright 2006-2007 Sun Microsystems, Inc.
*/
package org.opends.server.core;
@@ -1197,20 +1197,8 @@
}
- // Stop the processing timer.
- processingStopTime = System.currentTimeMillis();
-
-
- // Send the delete response to the client.
- getClientConnection().sendResponse(this);
-
-
- // Log the delete response.
- logDeleteResponse(this);
-
-
- // Notify any change listeners and/or persistent searches that might be
- // registered with the server.
+ // Notify any change notification listeners that might be registered with
+ // the server.
if (getResultCode() == ResultCode.SUCCESS)
{
for (ChangeNotificationListener changeListener :
@@ -1230,7 +1218,24 @@
message, msgID);
}
}
+ }
+
+ // Stop the processing timer.
+ processingStopTime = System.currentTimeMillis();
+
+
+ // Send the delete response to the client.
+ getClientConnection().sendResponse(this);
+
+
+ // Log the delete response.
+ logDeleteResponse(this);
+
+
+ // Notify any persistent searches that might be registered with the server.
+ if (getResultCode() == ResultCode.SUCCESS)
+ {
for (PersistentSearch persistentSearch :
DirectoryServer.getPersistentSearches())
{
diff --git a/opends/src/server/org/opends/server/core/GroupManager.java b/opends/src/server/org/opends/server/core/GroupManager.java
index 2331842..0806dea 100644
--- a/opends/src/server/org/opends/server/core/GroupManager.java
+++ b/opends/src/server/org/opends/server/core/GroupManager.java
@@ -62,6 +62,10 @@
import org.opends.server.types.SearchResultEntry;
import org.opends.server.types.SearchScope;
import org.opends.server.types.SearchFilter;
+import org.opends.server.types.operation.PostResponseAddOperation;
+import org.opends.server.types.operation.PostResponseDeleteOperation;
+import org.opends.server.types.operation.PostResponseModifyOperation;
+import org.opends.server.types.operation.PostResponseModifyDNOperation;
import static org.opends.server.config.ConfigConstants.*;
import static org.opends.server.loggers.Debug.*;
@@ -1186,7 +1190,7 @@
* a group definition, and if so it will be instantiated and registered with
* this group manager.
*/
- public void handleAddOperation(AddOperation addOperation,
+ public void handleAddOperation(PostResponseAddOperation addOperation,
Entry entry)
{
assert debugEnter(CLASS_NAME, "handleAddOperation",
@@ -1213,7 +1217,7 @@
* {@inheritDoc} In this case, if the entry is associated with a registered
* group instance, then that group instance will be deregistered.
*/
- public void handleDeleteOperation(DeleteOperation deleteOperation,
+ public void handleDeleteOperation(PostResponseDeleteOperation deleteOperation,
Entry entry)
{
assert debugEnter(CLASS_NAME, "handleDeleteOperation",
@@ -1241,7 +1245,7 @@
* group instance, then that instance will be recreated from the contents of
* the provided entry and re-registered with the group manager.
*/
- public void handleModifyOperation(ModifyOperation modifyOperation,
+ public void handleModifyOperation(PostResponseModifyOperation modifyOperation,
Entry oldEntry, Entry newEntry)
{
assert debugEnter(CLASS_NAME, "handleModifyOperation",
@@ -1285,7 +1289,7 @@
* DN, and the old instance will be deregistered.
*/
public void handleModifyDNOperation(
- ModifyDNOperation modifyDNOperation,
+ PostResponseModifyDNOperation modifyDNOperation,
Entry oldEntry, Entry newEntry)
{
assert debugEnter(CLASS_NAME, "handleModifyDNOperation",
diff --git a/opends/src/server/org/opends/server/core/ModifyDNOperation.java b/opends/src/server/org/opends/server/core/ModifyDNOperation.java
index 74cc0b8..8ec6126 100644
--- a/opends/src/server/org/opends/server/core/ModifyDNOperation.java
+++ b/opends/src/server/org/opends/server/core/ModifyDNOperation.java
@@ -2003,20 +2003,8 @@
}
- // Stop the processing timer.
- processingStopTime = System.currentTimeMillis();
-
-
- // Send the modify DN response to the client.
- clientConnection.sendResponse(this);
-
-
- // Log the modify DN response.
- logModifyDNResponse(this);
-
-
- // Notify any change listeners and/or persistent searches that might be
- // registered with the server.
+ // Notify any change notification listeners that might be registered with
+ // the server.
if (getResultCode() == ResultCode.SUCCESS)
{
for (ChangeNotificationListener changeListener :
@@ -2036,7 +2024,24 @@
message, msgID);
}
}
+ }
+
+ // Stop the processing timer.
+ processingStopTime = System.currentTimeMillis();
+
+
+ // Send the modify DN response to the client.
+ clientConnection.sendResponse(this);
+
+
+ // Log the modify DN response.
+ logModifyDNResponse(this);
+
+
+ // Notify any persistent searches that might be registered with the server.
+ if (getResultCode() == ResultCode.SUCCESS)
+ {
for (PersistentSearch persistentSearch :
DirectoryServer.getPersistentSearches())
{
diff --git a/opends/src/server/org/opends/server/core/ModifyOperation.java b/opends/src/server/org/opends/server/core/ModifyOperation.java
index 25ae482..333a9c4 100644
--- a/opends/src/server/org/opends/server/core/ModifyOperation.java
+++ b/opends/src/server/org/opends/server/core/ModifyOperation.java
@@ -2731,20 +2731,8 @@
}
- // Stop the processing timer.
- processingStopTime = System.currentTimeMillis();
-
-
- // Send the modify response to the client.
- clientConnection.sendResponse(this);
-
-
- // Log the modify response.
- logModifyResponse(this);
-
-
- // Notify any change listeners and/or persistent searches that might be
- // registered with the server.
+ // Notify any change notification listeners that might be registered with
+ // the server.
if (getResultCode() == ResultCode.SUCCESS)
{
for (ChangeNotificationListener changeListener :
@@ -2765,7 +2753,24 @@
message, msgID);
}
}
+ }
+
+ // Stop the processing timer.
+ processingStopTime = System.currentTimeMillis();
+
+
+ // Send the modify response to the client.
+ clientConnection.sendResponse(this);
+
+
+ // Log the modify response.
+ logModifyResponse(this);
+
+
+ // Notify any persistent searches that might be registered with the server.
+ if (getResultCode() == ResultCode.SUCCESS)
+ {
for (PersistentSearch persistentSearch :
DirectoryServer.getPersistentSearches())
{
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/core/TestChangeNotificationListener.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/core/TestChangeNotificationListener.java
index f5557fa..47e92e7 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/core/TestChangeNotificationListener.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/core/TestChangeNotificationListener.java
@@ -22,7 +22,7 @@
* CDDL HEADER END
*
*
- * Portions Copyright 2006 Sun Microsystems, Inc.
+ * Portions Copyright 2006-2007 Sun Microsystems, Inc.
*/
package org.opends.server.core;
@@ -32,6 +32,10 @@
import org.opends.server.api.ChangeNotificationListener;
import org.opends.server.types.Entry;
+import org.opends.server.types.operation.PostResponseAddOperation;
+import org.opends.server.types.operation.PostResponseDeleteOperation;
+import org.opends.server.types.operation.PostResponseModifyOperation;
+import org.opends.server.types.operation.PostResponseModifyDNOperation;
@@ -76,7 +80,7 @@
/**
* {@inheritDoc}
*/
- public void handleAddOperation(AddOperation addOperation,
+ public void handleAddOperation(PostResponseAddOperation addOperation,
Entry entry)
{
addCount.incrementAndGet();
@@ -87,7 +91,7 @@
/**
* {@inheritDoc}
*/
- public void handleDeleteOperation(DeleteOperation deleteOperation,
+ public void handleDeleteOperation(PostResponseDeleteOperation deleteOperation,
Entry entry)
{
deleteCount.incrementAndGet();
@@ -98,7 +102,7 @@
/**
* {@inheritDoc}
*/
- public void handleModifyOperation(ModifyOperation modifyOperation,
+ public void handleModifyOperation(PostResponseModifyOperation modifyOperation,
Entry oldEntry, Entry newEntry)
{
modifyCount.incrementAndGet();
@@ -110,7 +114,7 @@
* {@inheritDoc}
*/
public void handleModifyDNOperation(
- ModifyDNOperation modifyDNOperation,
+ PostResponseModifyDNOperation modifyDNOperation,
Entry oldEntry, Entry newEntry)
{
modifyDNCount.incrementAndGet();
--
Gitblit v1.10.0