Skip to content

Make AciList reads lock-free with true copy-on-write #674

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vharseko wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Make AciList reads lock-free with true copy-on-write #674

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fd019f6
Make AciList reads lock-free with true copy-on-write
vharseko Jul 3, 2026
1c17953
Update copyright information in AciList.java
vharseko Jul 3, 2026
b712f6e
AciList review follow-ups: plain mutator lock, COW regression test
vharseko Jul 4, 2026
f41cb71
Merge remote-tracking branch 'origin/master' into features/compare-1
vharseko Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 80 additions & 48 deletions opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/AciList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*
* Copyright 2008-2010 Sun Microsystems, Inc.
* Portions Copyright 2013-2016 ForgeRock AS.
* Portions Copyright 2026 3A Systems, LLC
*/
package org.opends.server.authorization.dseecompat;

Expand Down Expand Up @@ -48,12 +49,17 @@ public class AciList {

/**
* A map containing all the ACIs.
* We use the copy-on-write technique to avoid locking when reading.
* We use the copy-on-write technique to avoid locking when reading:
* mutators build a copy of the map (with copied value lists), modify the
* copy and publish it through this volatile reference. A published map is
* never modified in place, so getCandidateAcis() can read it lock-free —
* it runs for every access-controlled operation, and even a read lock
* becomes a cross-core hotspot under load.
*/
private volatile DITCacheMap<List<Aci>> aciList = new DITCacheMap<>();

/**
* Lock to protect internal data structures.
* Lock serializing mutators (ACI additions, removals and renames are rare).
*/
private final ReentrantReadWriteLock lock =
new ReentrantReadWriteLock();
Expand Down Expand Up @@ -87,46 +93,56 @@ public List<Aci> getCandidateAcis(DN baseDN) {
return candidates;
}

lock.readLock().lock();
try
{
//Save the baseDN in case we need to evaluate a global ACI.
DN entryDN=baseDN;
while (baseDN != null) {
List<Aci> acis = aciList.get(baseDN);
if (acis != null) {
//Check if there are global ACIs. Global ACI has a NULL DN.
if (baseDN.isRootDN()) {
for (Aci aci : acis) {
AciTargets targets = aci.getTargets();
//If there is a target, evaluate it to see if this ACI should
//be included in the candidate set.
if (targets != null
&& AciTargets.isTargetApplicable(aci, targets, entryDN))
{
candidates.add(aci); //Add this ACI to the candidates.
}
// Lock-free read of the copy-on-write map: mutators never modify a
// published map or its value lists in place.
final DITCacheMap<List<Aci>> aciList = this.aciList;
//Save the baseDN in case we need to evaluate a global ACI.
DN entryDN=baseDN;
while (baseDN != null) {
List<Aci> acis = aciList.get(baseDN);
if (acis != null) {
//Check if there are global ACIs. Global ACI has a NULL DN.
if (baseDN.isRootDN()) {
for (Aci aci : acis) {
AciTargets targets = aci.getTargets();
//If there is a target, evaluate it to see if this ACI should
//be included in the candidate set.
if (targets != null
&& AciTargets.isTargetApplicable(aci, targets, entryDN))
{
candidates.add(aci); //Add this ACI to the candidates.
}
} else {
candidates.addAll(acis);
}
}
if(baseDN.isRootDN()) {
break;
}
DN parentDN=baseDN.parent();
if(parentDN == null) {
baseDN=DN.rootDN();
} else {
baseDN=parentDN;
candidates.addAll(acis);
}
}
return candidates;
if(baseDN.isRootDN()) {
break;
}
DN parentDN=baseDN.parent();
if(parentDN == null) {
baseDN=DN.rootDN();
} else {
baseDN=parentDN;
}
}
finally
return candidates;
}

/**
* Returns a copy of the current ACI map with copied value lists, suitable
* for mutation and subsequent publication through the volatile reference.
* Must be called while holding the write lock.
*/
private DITCacheMap<List<Aci>> copyAciList()
{
DITCacheMap<List<Aci>> copy = new DITCacheMap<>();
for (Map.Entry<DN, List<Aci>> entry : aciList.entrySet())
{
lock.readLock().unlock();
copy.put(entry.getKey(), new LinkedList<>(entry.getValue()));
}
return copy;
}

/**
Expand All @@ -144,14 +160,16 @@ public int addAci(List<? extends Entry> entries,
lock.writeLock().lock();
try
{
DITCacheMap<List<Aci>> copy = copyAciList();
int validAcis = 0;
for (Entry entry : entries) {
DN dn=entry.getName();
List<Attribute> attributeList =
entry.getOperationalAttribute(AciHandler.aciType);
validAcis += addAciAttributeList(aciList, dn, configDN,
validAcis += addAciAttributeList(copy, dn, configDN,
attributeList, failedACIMsgs);
}
aciList = copy;
return validAcis;
}
finally
Expand All @@ -173,7 +191,9 @@ public void addAci(DN dn, SortedSet<Aci> acis) {
lock.writeLock().lock();
try
{
aciList.put(dn, new LinkedList<>(acis));
DITCacheMap<List<Aci>> copy = copyAciList();
copy.put(dn, new LinkedList<>(acis));
aciList = copy;
}
finally
{
Expand All @@ -198,21 +218,23 @@ public int addAci(Entry entry, boolean hasAci,
lock.writeLock().lock();
try
{
DITCacheMap<List<Aci>> copy = copyAciList();
int validAcis = 0;
//Process global "ds-cfg-global-aci" attribute type. The oldentry
//DN is checked to verify it is equal to the config DN. If not those
//attributes are skipped.
if(hasGlobalAci && entry.getName().equals(configDN)) {
List<Attribute> attributeList = entry.getAllAttributes(globalAciType);
validAcis = addAciAttributeList(aciList, DN.rootDN(), configDN,
validAcis = addAciAttributeList(copy, DN.rootDN(), configDN,
attributeList, failedACIMsgs);
}

if(hasAci) {
List<Attribute> attributeList = entry.getAllAttributes(aciType);
validAcis += addAciAttributeList(aciList, entry.getName(), configDN,
validAcis += addAciAttributeList(copy, entry.getName(), configDN,
attributeList, failedACIMsgs);
}
aciList = copy;
return validAcis;
}
finally
Expand Down Expand Up @@ -285,24 +307,26 @@ public void modAciOldNewEntry(Entry oldEntry, Entry newEntry,
lock.writeLock().lock();
try
{
DITCacheMap<List<Aci>> copy = copyAciList();
List<LocalizableMessage> failedACIMsgs=new LinkedList<>();
//Process "aci" attribute types.
if(hasAci) {
aciList.remove(oldEntry.getName());
copy.remove(oldEntry.getName());
List<Attribute> attributeList =
newEntry.getOperationalAttribute(aciType);
addAciAttributeList(aciList,newEntry.getName(), configDN,
addAciAttributeList(copy,newEntry.getName(), configDN,
attributeList, failedACIMsgs);
}
//Process global "ds-cfg-global-aci" attribute type. The oldentry
//DN is checked to verify it is equal to the config DN. If not those
//attributes are skipped.
if(hasGlobalAci && oldEntry.getName().equals(configDN)) {
aciList.remove(DN.rootDN());
copy.remove(DN.rootDN());
List<Attribute> attributeList = newEntry.getAllAttributes(globalAciType);
addAciAttributeList(aciList, DN.rootDN(), configDN,
addAciAttributeList(copy, DN.rootDN(), configDN,
attributeList, failedACIMsgs);
}
aciList = copy;
}
finally
{
Expand Down Expand Up @@ -346,17 +370,21 @@ public boolean removeAci(Entry entry, boolean hasAci,
lock.writeLock().lock();
try
{
DITCacheMap<List<Aci>> copy = copyAciList();
DN entryDN = entry.getName();
if (hasGlobalAci && entryDN.equals(configDN) &&
aciList.remove(DN.rootDN()) == null)
copy.remove(DN.rootDN()) == null)
{
aciList = copy;
return false;
}
boolean result = true;
if (hasAci || !hasGlobalAci)
{
return aciList.removeSubtree(entryDN, null);
result = copy.removeSubtree(entryDN, null);
}
return true;
aciList = copy;
return result;
}
finally
{
Expand All @@ -374,8 +402,9 @@ public void removeAci(LocalBackend<?> backend) {
lock.writeLock().lock();
try
{
DITCacheMap<List<Aci>> copy = copyAciList();
Iterator<Map.Entry<DN,List<Aci>>> iterator =
aciList.entrySet().iterator();
copy.entrySet().iterator();
while (iterator.hasNext())
{
Map.Entry<DN,List<Aci>> mapEntry = iterator.next();
Expand All @@ -384,6 +413,7 @@ public void removeAci(LocalBackend<?> backend) {
iterator.remove();
}
}
aciList = copy;
}
finally
{
Expand All @@ -402,9 +432,10 @@ public void renameAci(DN oldDN, DN newDN ) {
lock.writeLock().lock();
try
{
DITCacheMap<List<Aci>> copy = copyAciList();
Map<DN,List<Aci>> tempAciList = new HashMap<>();
Iterator<Map.Entry<DN,List<Aci>>> iterator =
aciList.entrySet().iterator();
copy.entrySet().iterator();
while (iterator.hasNext()) {
Map.Entry<DN,List<Aci>> hashEntry = iterator.next();
DN keyDn = hashEntry.getKey();
Expand All @@ -427,7 +458,8 @@ public void renameAci(DN oldDN, DN newDN ) {
iterator.remove();
}
}
aciList.putAll(tempAciList);
copy.putAll(tempAciList);
aciList = copy;
}
finally
{
Expand Down
Loading