[Pki-devel] [PATCH] 0057..0061 LDAPProfileSubsystem concurrency fixes
Fraser Tweedale
ftweedal at redhat.com
Wed Jan 20 02:07:28 UTC 2016
Rebase was needed for the remaining patches; 0060-2 and 0061-2
attached.
Thanks,
Fraser
-------------- next part --------------
From 2367b829f518a0003a461115fc2a905c0cc03a24 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at redhat.com>
Date: Mon, 30 Nov 2015 16:05:03 +1100
Subject: [PATCH 60/61] Handle LDAPProfileSubsystem delete-then-recreate races
Deleting and then immediately recreating a profile can result in the
new profile temporarily going missing, if the DELETE
EntryChangeControl is processed after profile readdition.
Handle this case by tracking the nsUniqueId of entries that are
deleted by an LDAPProfileSubsystem and NOT (re-)forgetting the
profile when the subsequent EntryChangeControl gets processed.
Fixes: https://fedorahosted.org/pki/ticket/1700
---
.../cmscore/profile/LDAPProfileSubsystem.java | 112 +++++++++++++++++----
1 file changed, 92 insertions(+), 20 deletions(-)
diff --git a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
index b16a33fe25881956051e2f65a5a99b73f7a624b4..f48aea3918e6fc4a08c9bc90a433b834a29f35d3 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
@@ -22,6 +22,7 @@ import java.io.InputStream;
import java.util.Hashtable;
import java.util.LinkedHashMap;
import java.util.TreeMap;
+import java.util.TreeSet;
import netscape.ldap.LDAPAttribute;
import netscape.ldap.LDAPConnection;
@@ -62,6 +63,11 @@ public class LDAPProfileSubsystem
* of the profile entry that this instance has seen */
private TreeMap<String,Integer> entryUSNs;
+ private TreeMap<String,String> nsUniqueIds;
+
+ /* Set of nsUniqueIds of deleted entries */
+ private TreeSet<String> deletedNsUniqueIds;
+
/**
* Initializes this subsystem with the given configuration
* store.
@@ -79,6 +85,8 @@ public class LDAPProfileSubsystem
mProfiles = new LinkedHashMap<String, IProfile>();
mProfileClassIds = new Hashtable<String, String>();
entryUSNs = new TreeMap<>();
+ nsUniqueIds = new TreeMap<>();
+ deletedNsUniqueIds = new TreeSet<>();
IConfigStore cs = CMS.getConfigStore();
IConfigStore dbCfg = cs.getSubStore("internaldb");
@@ -110,6 +118,14 @@ public class LDAPProfileSubsystem
IPluginRegistry registry = (IPluginRegistry)
CMS.getSubsystem(CMS.SUBSYSTEM_REGISTRY);
+ String nsUniqueId =
+ ldapProfile.getAttribute("nsUniqueId").getStringValueArray()[0];
+ if (deletedNsUniqueIds.contains(nsUniqueId)) {
+ CMS.debug("readProfile: ignoring entry with nsUniqueId '"
+ + nsUniqueId + "' due to deletion");
+ return;
+ }
+
String profileId = null;
String dn = ldapProfile.getDN();
if (!dn.startsWith("cn=")) {
@@ -145,6 +161,7 @@ public class LDAPProfileSubsystem
CMS.debug("Start Profile Creation - " + profileId + " " + classId + " " + info.getClassName());
createProfile(profileId, classId, info.getClassName(), data);
entryUSNs.put(profileId, newEntryUSN);
+ nsUniqueIds.put(profileId, nsUniqueId);
CMS.debug("Done Profile Creation - " + profileId);
} catch (EProfileException e) {
CMS.debug("Error creating profile '" + profileId + "'; skipping.");
@@ -192,7 +209,7 @@ public class LDAPProfileSubsystem
}
}
- public void deleteProfile(String id) throws EProfileException {
+ public synchronized void deleteProfile(String id) throws EProfileException {
if (isProfileEnable(id)) {
throw new EProfileException("CMS_PROFILE_DELETE_ENABLEPROFILE");
}
@@ -215,9 +232,31 @@ public class LDAPProfileSubsystem
}
}
+ deletedNsUniqueIds.add(nsUniqueIds.get(id));
forgetProfile(id);
}
+ private synchronized void handleDELETE(LDAPEntry entry) {
+ LDAPAttribute attr = entry.getAttribute("nsUniqueId");
+ String nsUniqueId = null;
+ if (attr != null)
+ nsUniqueId = attr.getStringValueArray()[0];
+
+ if (deletedNsUniqueIds.remove(nsUniqueId)) {
+ CMS.debug("handleDELETE: delete was already effected");
+ return;
+ }
+
+ String profileId = null;
+ String dn = entry.getDN();
+ if (!dn.startsWith("cn=")) {
+ CMS.debug("handleDELETE: DN " + dn + " does not start with 'cn='");
+ return;
+ }
+ profileId = LDAPDN.explodeDN(dn, true)[0];
+ forgetProfile(profileId);
+ }
+
private synchronized void handleMODDN(DN oldDN, LDAPEntry entry) {
DN profilesDN = new DN(dn);
@@ -231,20 +270,61 @@ public class LDAPProfileSubsystem
@Override
public synchronized void commitProfile(String id) throws EProfileException {
LDAPConfigStore cs = (LDAPConfigStore) mProfiles.get(id).getConfigStore();
+
+ // first create a *new* profile object from the configStore
+ // and initialise it with the updated configStore
+ //
+ IPluginRegistry registry = (IPluginRegistry)
+ CMS.getSubsystem(CMS.SUBSYSTEM_REGISTRY);
+ String classId = mProfileClassIds.get(id);
+ IPluginInfo info = registry.getPluginInfo("profile", classId);
+ String className = info.getClassName();
+ IProfile newProfile = null;
try {
- String[] attrs = {"entryUSN"};
+ newProfile = (IProfile) Class.forName(className).newInstance();
+ } catch (ClassNotFoundException | InstantiationException | IllegalAccessException e) {
+ throw new EProfileException("Could not instantiate class '"
+ + classId + "' for profile '" + id + "': " + e);
+ }
+ newProfile.setId(id);
+ try {
+ newProfile.init(this, cs);
+ } catch (EBaseException e) {
+ throw new EProfileException(
+ "Failed to initialise profile '" + id + "': " + e);
+ }
+
+ // next replace the existing profile with the new profile;
+ // this is to avoid any intermediate state where the profile
+ // is not fully initialised with its inputs, outputs and
+ // policy objects.
+ //
+ mProfiles.put(id, newProfile);
+
+ // finally commit the configStore and track the resulting
+ // entryUSN and (in case of add) the nsUniqueId
+ //
+ try {
+ String[] attrs = {"entryUSN", "nsUniqueId"};
LDAPEntry entry = cs.commitReturn(false, attrs);
-
- LDAPAttribute attr = null;
- if (entry != null)
- attr = entry.getAttribute("entryUSN");
+ if (entry == null) {
+ // shouldn't happen, but let's be sure not to crash anyway
+ return;
+ }
Integer entryUSN = null;
+ LDAPAttribute attr = entry.getAttribute("entryUSN");
if (attr != null)
entryUSN = new Integer(attr.getStringValueArray()[0]);
-
entryUSNs.put(id, entryUSN);
CMS.debug("commitProfile: new entryUSN = " + entryUSN);
+
+ String nsUniqueId = null;
+ attr = entry.getAttribute("nsUniqueId");
+ if (attr != null)
+ nsUniqueId = attr.getStringValueArray()[0];
+ CMS.debug("commitProfile: nsUniqueId = " + nsUniqueId);
+ nsUniqueIds.put(id, nsUniqueId);
} catch (ELdapException e) {
throw new EProfileException(
"Failed to commit config store of profile '" + id + ": " + e);
@@ -261,17 +341,7 @@ public class LDAPProfileSubsystem
mProfiles.remove(id);
mProfileClassIds.remove(id);
entryUSNs.remove(id);
- }
-
- private void forgetProfile(LDAPEntry entry) {
- String profileId = null;
- String dn = entry.getDN();
- if (!dn.startsWith("cn=")) {
- CMS.debug("forgetProfile: DN " + dn + " does not start with 'cn='");
- return;
- }
- profileId = LDAPDN.explodeDN(dn, true)[0];
- forgetProfile(profileId);
+ nsUniqueIds.remove(id);
}
/**
@@ -296,6 +366,8 @@ public class LDAPProfileSubsystem
mProfiles.clear();
mProfileClassIds.clear();
entryUSNs.clear();
+ nsUniqueIds.clear();
+ deletedNsUniqueIds.clear();
}
/**
@@ -328,7 +400,7 @@ public class LDAPProfileSubsystem
cons.setServerControls(persistCtrl);
cons.setBatchSize(1);
cons.setServerTimeLimit(0 /* seconds */);
- String[] attrs = {"*", "entryUSN"};
+ String[] attrs = {"*", "entryUSN", "nsUniqueId"};
LDAPSearchResults results = conn.search(
dn, LDAPConnection.SCOPE_ONE, "(objectclass=*)",
attrs, false, cons);
@@ -347,7 +419,7 @@ public class LDAPProfileSubsystem
break;
case LDAPPersistSearchControl.DELETE:
CMS.debug("Profile change monitor: DELETE");
- forgetProfile(entry);
+ handleDELETE(entry);
break;
case LDAPPersistSearchControl.MODIFY:
CMS.debug("Profile change monitor: MODIFY");
--
2.5.0
-------------- next part --------------
From 40eb5e91d1140caa6313ec6d9aa69d3b6f75b9fa Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at redhat.com>
Date: Tue, 1 Dec 2015 14:21:08 +1100
Subject: [PATCH 61/61] Ensure config store commits refresh file-based profile
data
The file-based LDAP profile subsystem does not update profiles
correctly. Ensure that each commit of the underlying config store
refreshes the profile inputs, outputs and policy objects.
Part of: https://fedorahosted.org/pki/ticket/1700
---
.../cmscore/profile/AbstractProfileSubsystem.java | 39 +++++++++++++++++++++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java
index e93e090de9244a3a4ad4f17e0616cce5d65a978c..116b8e2026e80b012fb87647fd8924b567194fa3 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java
@@ -22,12 +22,15 @@ import java.util.Enumeration;
import java.util.Hashtable;
import java.util.LinkedHashMap;
+import com.netscape.certsrv.apps.CMS;
import com.netscape.certsrv.base.EBaseException;
import com.netscape.certsrv.base.IConfigStore;
import com.netscape.certsrv.base.ISubsystem;
import com.netscape.certsrv.profile.EProfileException;
import com.netscape.certsrv.profile.IProfile;
import com.netscape.certsrv.profile.IProfileSubsystem;
+import com.netscape.certsrv.registry.IPluginInfo;
+import com.netscape.certsrv.registry.IPluginRegistry;
public abstract class AbstractProfileSubsystem implements IProfileSubsystem {
protected static final String PROP_CHECK_OWNER = "checkOwner";
@@ -120,8 +123,42 @@ public abstract class AbstractProfileSubsystem implements IProfileSubsystem {
*/
public void commitProfile(String id)
throws EProfileException {
+ IConfigStore cs = mProfiles.get(id).getConfigStore();
+
+ // first create a *new* profile object from the configStore
+ // and initialise it with the updated configStore
+ //
+ IPluginRegistry registry = (IPluginRegistry)
+ CMS.getSubsystem(CMS.SUBSYSTEM_REGISTRY);
+ String classId = mProfileClassIds.get(id);
+ IPluginInfo info = registry.getPluginInfo("profile", classId);
+ String className = info.getClassName();
+ IProfile newProfile = null;
try {
- mProfiles.get(id).getConfigStore().commit(false);
+ newProfile = (IProfile) Class.forName(className).newInstance();
+ } catch (ClassNotFoundException | InstantiationException | IllegalAccessException e) {
+ throw new EProfileException("Could not instantiate class '"
+ + classId + "' for profile '" + id + "': " + e);
+ }
+ newProfile.setId(id);
+ try {
+ newProfile.init(this, cs);
+ } catch (EBaseException e) {
+ throw new EProfileException(
+ "Failed to initialise profile '" + id + "': " + e);
+ }
+
+ // next replace the existing profile with the new profile;
+ // this is to avoid any intermediate state where the profile
+ // is not fully initialised with its inputs, outputs and
+ // policy objects.
+ //
+ mProfiles.put(id, newProfile);
+
+ // finally commit the configStore
+ //
+ try {
+ cs.commit(false);
} catch (EBaseException e) {
throw new EProfileException(
"Failed to commit config store of profile '" + id + ": " + e,
--
2.5.0
More information about the Pki-devel
mailing list