Skip to content
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

Jump to bottom

Refactor PlatformManager to use locks #2571

Open
wants to merge 4 commits into
base: version/7.3.x
Choose a base branch
from
+87 −23
Open

Refactor PlatformManager to use locks #2571

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cdf5fc9
Refactor PlatformManager to use locks
me4502 Jul 7, 2024
0ac7a04
fix a case of unnecessary reentrant locking
me4502 Jul 7, 2024
a27bdf2
keep the platform lock until preferences are updated in #unregister
me4502 Jul 7, 2024
83f7b2b
Use StampedLocks for performance reasons
me4502 Jul 7, 2024
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
110 changes: 87 additions & 23 deletions worldedit-core/src/main/java/com/sk89q/worldedit/extension/platform/PlatformManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.StampedLock;
import javax.annotation.Nullable;

import static com.google.common.base.Preconditions.checkNotNull;
Expand All @@ -75,6 +76,9 @@ public class PlatformManager {
private final AtomicBoolean initialized = new AtomicBoolean();
private final AtomicBoolean configured = new AtomicBoolean();

private final StampedLock platformsLock = new StampedLock();
private final StampedLock preferencesLock = new StampedLock();

/**
* Create a new platform manager.
*
Expand All @@ -94,14 +98,19 @@ public PlatformManager(WorldEdit worldEdit) {
*
* @param platform the platform
*/
public synchronized void register(Platform platform) {
public void register(Platform platform) {
checkNotNull(platform);

LOGGER.info("Got request to register " + platform.getClass() + " with WorldEdit [" + super.toString() + "]");

// Just add the platform to the list of platforms: we'll pick favorites
// once all the platforms have been loaded
platforms.add(platform);
long stamp = platformsLock.writeLock();
try {
platforms.add(platform);
} finally {
platformsLock.unlockWrite(stamp);
}

// Make sure that versions are in sync
if (firstSeenVersion != null) {
Expand All @@ -123,26 +132,39 @@ public synchronized void register(Platform platform) {
*
* @param platform the platform
*/
public synchronized boolean unregister(Platform platform) {
public boolean unregister(Platform platform) {
checkNotNull(platform);

boolean removed = platforms.remove(platform);
boolean removed;
long platformsStamp = platformsLock.writeLock();

try {
removed = platforms.remove(platform);
} finally {
platformsLock.unlockWrite(platformsStamp);
}

if (removed) {
LOGGER.info("Unregistering " + platform.getClass().getCanonicalName() + " from WorldEdit");

boolean choosePreferred = false;

// Check whether this platform was chosen to be the preferred one
// for any capability and be sure to remove it
Iterator<Entry<Capability, Platform>> it = preferences.entrySet().iterator();
while (it.hasNext()) {
Entry<Capability, Platform> entry = it.next();
if (entry.getValue().equals(platform)) {
entry.getKey().uninitialize(this, entry.getValue());
it.remove();
choosePreferred = true; // Have to choose new favorites
long preferencesStamp = preferencesLock.writeLock();

try {
// Check whether this platform was chosen to be the preferred one
// for any capability and be sure to remove it
Iterator<Entry<Capability, Platform>> it = preferences.entrySet().iterator();
while (it.hasNext()) {
Entry<Capability, Platform> entry = it.next();
if (entry.getValue().equals(platform)) {
entry.getKey().uninitialize(this, entry.getValue());
it.remove();
choosePreferred = true; // Have to choose new favorites
}
}
} finally {
preferencesLock.unlockWrite(preferencesStamp);
}

if (choosePreferred) {
Expand All @@ -160,12 +182,27 @@ public synchronized boolean unregister(Platform platform) {
* @return the platform
* @throws NoCapablePlatformException thrown if no platform is capable
*/
public synchronized Platform queryCapability(Capability capability) throws NoCapablePlatformException {
Platform platform = preferences.get(checkNotNull(capability));
public Platform queryCapability(Capability capability) throws NoCapablePlatformException {
checkNotNull(capability);

long stamp = preferencesLock.tryOptimisticRead();
Platform platform = preferences.get(capability);
boolean hasNoPreferences = platform == null && preferences.isEmpty();

if (!preferencesLock.validate(stamp)) {
stamp = preferencesLock.readLock();
try {
platform = preferences.get(capability);
hasNoPreferences = platform == null && preferences.isEmpty();
} finally {
preferencesLock.unlockRead(stamp);
}
}

if (platform != null) {
return platform;
} else {
if (preferences.isEmpty()) {
if (hasNoPreferences) {
// Not all platforms registered, this is being called too early!
throw new NoCapablePlatformException(
"Not all platforms have been registered yet!"
Expand All @@ -179,11 +216,17 @@ public synchronized Platform queryCapability(Capability capability) throws NoCap
/**
* Choose preferred platforms and perform necessary initialization.
*/
private synchronized void choosePreferred() {
private void choosePreferred() {
for (Capability capability : Capability.values()) {
Platform preferred = findMostPreferred(capability);
if (preferred != null) {
Platform oldPreferred = preferences.put(capability, preferred);
Platform oldPreferred;
long stamp = preferencesLock.writeLock();
try {
oldPreferred = preferences.put(capability, preferred);
} finally {
preferencesLock.unlockWrite(stamp);
}
// only (re)initialize if it changed
if (preferred != oldPreferred) {
// uninitialize if needed
Expand All @@ -195,8 +238,18 @@ private synchronized void choosePreferred() {
}
}

long stamp = preferencesLock.tryOptimisticRead();
boolean hasConfiguration = preferences.containsKey(Capability.CONFIGURATION);
if (!preferencesLock.validate(stamp)) {
stamp = preferencesLock.readLock();
try {
hasConfiguration = preferences.containsKey(Capability.CONFIGURATION);
} finally {
preferencesLock.unlockRead(stamp);
}
}
// Fire configuration event
if (preferences.containsKey(Capability.CONFIGURATION) && configured.compareAndSet(false, true)) {
if (hasConfiguration && configured.compareAndSet(false, true)) {
worldEdit.getEventBus().post(new ConfigurationLoadEvent(queryCapability(Capability.CONFIGURATION).getConfiguration()));
}
}
Expand All @@ -208,11 +261,11 @@ private synchronized void choosePreferred() {
* @param capability the capability
* @return the most preferred platform, or null if no platform was found
*/
private synchronized @Nullable Platform findMostPreferred(Capability capability) {
private @Nullable Platform findMostPreferred(Capability capability) {
Platform preferred = null;
Preference highest = null;

for (Platform platform : platforms) {
for (Platform platform : getPlatforms()) {
Preference preference = platform.getCapabilities().get(capability);
if (preference != null && (highest == null || preference.isPreferredOver(highest))) {
preferred = platform;
Expand All @@ -230,8 +283,19 @@ private synchronized void choosePreferred() {
*
* @return a list of platforms
*/
public synchronized List<Platform> getPlatforms() {
return new ArrayList<>(platforms);
public List<Platform> getPlatforms() {
long stamp = platformsLock.tryOptimisticRead();
List<Platform> platformsCopy = new ArrayList<>(platforms);
if (!platformsLock.validate(stamp)) {
stamp = platformsLock.readLock();
try {
platformsCopy = new ArrayList<>(platforms);
} finally {
platformsLock.unlockRead(stamp);
}
}

return platformsCopy;
}

/**
Expand Down