From 1cc3bd90b688e7bc65d5e7ce79ccadeba3ede628 Mon Sep 17 00:00:00 2001
From: Chris Ridd <chris.ridd@forgerock.com>
Date: Thu, 28 Jan 2016 13:47:41 +0000
Subject: [PATCH] OPENDJ-1906: Protect StaticGroups with a RWLock to allow updating in-place
---
opendj-server-legacy/src/main/java/org/opends/server/extensions/StaticGroup.java | 213 +++++++++++++++++++++++++++++++++++++++++++---------
1 files changed, 175 insertions(+), 38 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/extensions/StaticGroup.java b/opendj-server-legacy/src/main/java/org/opends/server/extensions/StaticGroup.java
index 64f9592..ca65fdc 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/extensions/StaticGroup.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/extensions/StaticGroup.java
@@ -31,6 +31,8 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Set;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.forgerock.i18n.LocalizableMessage;
import org.forgerock.i18n.LocalizedIllegalArgumentException;
@@ -42,6 +44,7 @@
import org.forgerock.opendj.ldap.ModificationType;
import org.forgerock.opendj.ldap.ResultCode;
import org.forgerock.opendj.ldap.SearchScope;
+import org.forgerock.util.Reject;
import org.opends.server.admin.std.server.GroupImplementationCfg;
import org.opends.server.admin.std.server.StaticGroupImplementationCfg;
import org.opends.server.api.Group;
@@ -104,6 +107,9 @@
/** Passed to the group manager to see if the nested group list needs to be refreshed. */
private long nestedGroupRefreshToken = DirectoryServer.getGroupManager().refreshToken();
+ /** Read/write lock protecting memberDNs and nestedGroups. */
+ private ReadWriteLock lock = new ReentrantReadWriteLock();
+
private ServerContext serverContext;
/**
@@ -129,7 +135,7 @@
LinkedHashSet<CompactDn> memberDNs)
{
super();
- ifNull(groupEntryDN, memberAttributeType, memberDNs);
+ Reject.ifNull(groupEntryDN, memberAttributeType, memberDNs);
this.serverContext = serverContext;
this.groupEntryDN = groupEntryDN;
@@ -149,7 +155,7 @@
@Override
public StaticGroup newInstance(ServerContext serverContext, Entry groupEntry) throws DirectoryException
{
- ifNull(groupEntry);
+ Reject.ifNull(groupEntry);
// Determine whether it is a groupOfNames, groupOfEntries or
// groupOfUniqueNames entry. If not, then that's a problem.
@@ -239,7 +245,7 @@
@Override
public boolean isGroupDefinition(Entry entry)
{
- ifNull(entry);
+ Reject.ifNull(entry);
// FIXME -- This needs to exclude enhanced groups once we have support for them.
if (hasObjectClass(entry, OC_VIRTUAL_STATIC_GROUP))
@@ -295,12 +301,23 @@
@Override
public List<DN> getNestedGroupDNs()
{
- try {
+ try
+ {
reloadIfNeeded();
- } catch (DirectoryException ex) {
+ }
+ catch (DirectoryException ex)
+ {
return Collections.<DN>emptyList();
}
- return nestedGroups;
+ lock.readLock().lock();
+ try
+ {
+ return nestedGroups;
+ }
+ finally
+ {
+ lock.readLock().unlock();
+ }
}
/** {@inheritDoc} */
@@ -308,9 +325,10 @@
public void addNestedGroup(DN nestedGroupDN)
throws UnsupportedOperationException, DirectoryException
{
- ifNull(nestedGroupDN);
+ Reject.ifNull(nestedGroupDN);
- synchronized (this)
+ lock.writeLock().lock();
+ try
{
if (nestedGroups.contains(nestedGroupDN))
{
@@ -335,6 +353,10 @@
newMemberDNs.add(toCompactDn(nestedGroupDN));
memberDNs = newMemberDNs;
}
+ finally
+ {
+ lock.writeLock().unlock();
+ }
}
/** {@inheritDoc} */
@@ -342,9 +364,10 @@
public void removeNestedGroup(DN nestedGroupDN)
throws UnsupportedOperationException, DirectoryException
{
- ifNull(nestedGroupDN);
+ Reject.ifNull(nestedGroupDN);
- synchronized (this)
+ lock.writeLock().lock();
+ try
{
if (! nestedGroups.contains(nestedGroupDN))
{
@@ -369,6 +392,10 @@
newMemberDNs.remove(toCompactDn(nestedGroupDN));
memberDNs = newMemberDNs;
}
+ finally
+ {
+ lock.writeLock().unlock();
+ }
}
/** {@inheritDoc} */
@@ -377,25 +404,33 @@
{
reloadIfNeeded();
CompactDn compactUserDN = toCompactDn(userDN);
- if (memberDNs.contains(compactUserDN))
+ lock.readLock().lock();
+ try
{
- return true;
- }
- else if (!examinedGroups.add(getGroupDN()))
- {
- return false;
- }
- else
- {
- for(DN nestedGroupDN : nestedGroups)
+ if (memberDNs.contains(compactUserDN))
{
- Group<? extends GroupImplementationCfg> group = getGroupManager().getGroupInstance(nestedGroupDN);
- if (group != null && group.isMember(userDN, examinedGroups))
+ return true;
+ }
+ else if (!examinedGroups.add(getGroupDN()))
+ {
+ return false;
+ }
+ else
+ {
+ for (DN nestedGroupDN : nestedGroups)
{
- return true;
+ Group<? extends GroupImplementationCfg> group = getGroupManager().getGroupInstance(nestedGroupDN);
+ if (group != null && group.isMember(userDN, examinedGroups))
+ {
+ return true;
+ }
}
}
}
+ finally
+ {
+ lock.readLock().unlock();
+ }
return false;
}
@@ -417,14 +452,18 @@
//the current token.
if (DirectoryServer.getGroupManager().hasInstancesChanged(nestedGroupRefreshToken))
{
- synchronized (this)
+ lock.writeLock().lock();
+ try
{
Group<?> thisGroup = DirectoryServer.getGroupManager().getGroupInstance(groupEntryDN);
// Check if the group itself has been removed
- if (thisGroup == null) {
+ if (thisGroup == null)
+ {
throw new DirectoryException(ResultCode.NO_SUCH_ATTRIBUTE,
ERR_STATICGROUP_GROUP_INSTANCE_INVALID.get(groupEntryDN));
- } else if (thisGroup != this) {
+ }
+ else if (thisGroup != this)
+ {
LinkedHashSet<CompactDn> newMemberDNs = new LinkedHashSet<>();
MemberList memberList = thisGroup.getMembers();
while (memberList.hasMoreMembers())
@@ -440,18 +479,21 @@
}
memberDNs = newMemberDNs;
}
- LinkedList<DN> newNestedGroups = new LinkedList<>();
+ nestedGroups.clear();
for (CompactDn compactDn : memberDNs)
{
DN dn = fromCompactDn(compactDn);
Group<?> group = DirectoryServer.getGroupManager().getGroupInstance(dn);
if (group != null)
{
- newNestedGroups.add(group.getGroupDN());
+ nestedGroups.add(group.getGroupDN());
}
}
nestedGroupRefreshToken = DirectoryServer.getGroupManager().refreshToken();
- nestedGroups=newNestedGroups;
+ }
+ finally
+ {
+ lock.writeLock().unlock();
}
}
}
@@ -461,7 +503,15 @@
public MemberList getMembers() throws DirectoryException
{
reloadIfNeeded();
- return new SimpleStaticGroupMemberList(groupEntryDN, memberDNs);
+ lock.readLock().lock();
+ try
+ {
+ return new SimpleStaticGroupMemberList(groupEntryDN, memberDNs);
+ }
+ finally
+ {
+ lock.readLock().unlock();
+ }
}
/** {@inheritDoc} */
@@ -469,11 +519,19 @@
public MemberList getMembers(DN baseDN, SearchScope scope, SearchFilter filter) throws DirectoryException
{
reloadIfNeeded();
- if (baseDN == null && filter == null)
+ lock.readLock().lock();
+ try
{
- return new SimpleStaticGroupMemberList(groupEntryDN, memberDNs);
+ if (baseDN == null && filter == null)
+ {
+ return new SimpleStaticGroupMemberList(groupEntryDN, memberDNs);
+ }
+ return new FilteredStaticGroupMemberList(groupEntryDN, memberDNs, baseDN, scope, filter);
}
- return new FilteredStaticGroupMemberList(groupEntryDN, memberDNs, baseDN, scope, filter);
+ finally
+ {
+ lock.readLock().unlock();
+ }
}
/** {@inheritDoc} */
@@ -485,11 +543,80 @@
/** {@inheritDoc} */
@Override
+ public void updateMembers(List<Modification> modifications)
+ throws UnsupportedOperationException, DirectoryException
+ {
+ Reject.ifNull(memberDNs);
+ Reject.ifNull(nestedGroups);
+
+ reloadIfNeeded();
+ lock.writeLock().lock();
+ try
+ {
+ for (Modification mod : modifications)
+ {
+ Attribute attribute = mod.getAttribute();
+ if (attribute.getAttributeType().equals(memberAttributeType))
+ {
+ switch (mod.getModificationType().asEnum())
+ {
+ case ADD:
+ for (ByteString v : attribute)
+ {
+ DN member = DN.decode(v);
+ memberDNs.add(toCompactDn(member));
+ if (DirectoryServer.getGroupManager().getGroupInstance(member) != null)
+ {
+ nestedGroups.add(member);
+ }
+ }
+ break;
+ case DELETE:
+ if (attribute.isEmpty())
+ {
+ memberDNs.clear();
+ nestedGroups.clear();
+ }
+ else
+ {
+ for (ByteString v : attribute)
+ {
+ DN member = DN.decode(v);
+ memberDNs.remove(toCompactDn(member));
+ nestedGroups.remove(member);
+ }
+ }
+ break;
+ case REPLACE:
+ memberDNs.clear();
+ nestedGroups.clear();
+ for (ByteString v : attribute)
+ {
+ DN member = DN.decode(v);
+ memberDNs.add(toCompactDn(member));
+ if (DirectoryServer.getGroupManager().getGroupInstance(member) != null)
+ {
+ nestedGroups.add(member);
+ }
+ }
+ break;
+ }
+ }
+ }
+ }
+ finally {
+ lock.writeLock().unlock();
+ }
+ }
+
+ /** {@inheritDoc} */
+ @Override
public void addMember(Entry userEntry) throws UnsupportedOperationException, DirectoryException
{
- ifNull(userEntry);
+ Reject.ifNull(userEntry);
- synchronized (this)
+ lock.writeLock().lock();
+ try
{
DN userDN = userEntry.getName();
CompactDn compactUserDN = toCompactDn(userDN);
@@ -512,16 +639,21 @@
newMemberDNs.add(compactUserDN);
memberDNs = newMemberDNs;
}
+ finally
+ {
+ lock.writeLock().unlock();
+ }
}
/** {@inheritDoc} */
@Override
public void removeMember(DN userDN) throws UnsupportedOperationException, DirectoryException
{
- ifNull(userDN);
+ Reject.ifNull(userDN);
CompactDn compactUserDN = toCompactDn(userDN);
- synchronized (this)
+ lock.writeLock().lock();
+ try
{
if (! memberDNs.contains(compactUserDN))
{
@@ -541,12 +673,17 @@
newMemberDNs.remove(compactUserDN);
memberDNs = newMemberDNs;
//If it is in the nested group list remove it.
- if(nestedGroups.contains(userDN)) {
+ if (nestedGroups.contains(userDN))
+ {
LinkedList<DN> newNestedGroups = new LinkedList<>(nestedGroups);
newNestedGroups.remove(userDN);
nestedGroups = newNestedGroups;
}
}
+ finally
+ {
+ lock.writeLock().unlock();
+ }
}
private ModifyOperation newModifyOperation(ModificationType modType, DN userDN)
--
Gitblit v1.10.0