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

Thread safe PluginManager #3771

Closed
wants to merge 2 commits into from
Closed

Thread safe PluginManager #3771

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
856cfc1
Make PluginManager thread safe
BoomEaro Jan 7, 2025
8d541fa
Separate locks for commands and listeners
BoomEaro Jan 25, 2025
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
105 changes: 84 additions & 21 deletions api/src/main/java/net/md_5/bungee/api/plugin/PluginManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
import java.util.Map;
import java.util.Set;
import java.util.Stack;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
import java.util.logging.Level;
Expand Down Expand Up @@ -59,6 +63,9 @@ public final class PluginManager
private final Multimap<Plugin, Command> commandsByPlugin = ArrayListMultimap.create();
private final Multimap<Plugin, Listener> listenersByPlugin = ArrayListMultimap.create();

private final ReadWriteLock commandsLock = new ReentrantReadWriteLock();
private final Lock listenersLock = new ReentrantLock();

@SuppressWarnings("unchecked")
public PluginManager(ProxyServer proxy)
{
Expand Down Expand Up @@ -93,12 +100,19 @@ public PluginManager(ProxyServer proxy)
*/
public void registerCommand(Plugin plugin, Command command)
{
commandMap.put( command.getName().toLowerCase( Locale.ROOT ), command );
for ( String alias : command.getAliases() )
commandsLock.writeLock().lock();
try
{
commandMap.put( command.getName().toLowerCase( Locale.ROOT ), command );
for ( String alias : command.getAliases() )
{
commandMap.put( alias.toLowerCase( Locale.ROOT ), command );
}
commandsByPlugin.put( plugin, command );
} finally
{
commandMap.put( alias.toLowerCase( Locale.ROOT ), command );
commandsLock.writeLock().unlock();
}
commandsByPlugin.put( plugin, command );
}

/**
Expand All @@ -108,8 +122,15 @@ public void registerCommand(Plugin plugin, Command command)
*/
public void unregisterCommand(Command command)
{
while ( commandMap.values().remove( command ) );
commandsByPlugin.values().remove( command );
commandsLock.writeLock().lock();
try
{
while ( commandMap.values().remove( command ) );
commandsByPlugin.values().remove( command );
} finally
{
commandsLock.writeLock().unlock();
}
}

/**
Expand All @@ -119,11 +140,18 @@ public void unregisterCommand(Command command)
*/
public void unregisterCommands(Plugin plugin)
{
for ( Iterator<Command> it = commandsByPlugin.get( plugin ).iterator(); it.hasNext(); )
commandsLock.writeLock().lock();
try
{
Command command = it.next();
while ( commandMap.values().remove( command ) );
it.remove();
for ( Iterator<Command> it = commandsByPlugin.get( plugin ).iterator(); it.hasNext(); )
{
Command command = it.next();
while ( commandMap.values().remove( command ) );
it.remove();
}
} finally
{
commandsLock.writeLock().unlock();
}
}

Expand All @@ -137,7 +165,14 @@ private Command getCommandIfEnabled(String commandName, CommandSender sender)
return null;
}

return commandMap.get( commandLower );
commandsLock.readLock().lock();
try
{
return commandMap.get( commandLower );
} finally
{
commandsLock.readLock().unlock();
}
}

/**
Expand Down Expand Up @@ -434,13 +469,20 @@ public <T extends Event> T callEvent(T event)
*/
public void registerListener(Plugin plugin, Listener listener)
{
for ( Method method : listener.getClass().getDeclaredMethods() )
listenersLock.lock();
try
{
Preconditions.checkArgument( !method.isAnnotationPresent( Subscribe.class ),
for ( Method method : listener.getClass().getDeclaredMethods() )
{
Preconditions.checkArgument( !method.isAnnotationPresent( Subscribe.class ),
"Listener %s has registered using deprecated subscribe annotation! Please update to @EventHandler.", listener );
}
eventBus.register( listener );
listenersByPlugin.put( plugin, listener );
} finally
{
listenersLock.unlock();
}
eventBus.register( listener );
listenersByPlugin.put( plugin, listener );
}

/**
Expand All @@ -450,8 +492,15 @@ public void registerListener(Plugin plugin, Listener listener)
*/
public void unregisterListener(Listener listener)
{
eventBus.unregister( listener );
listenersByPlugin.values().remove( listener );
listenersLock.lock();
try
{
eventBus.unregister( listener );
listenersByPlugin.values().remove( listener );
} finally
{
listenersLock.unlock();
}
}

/**
Expand All @@ -461,10 +510,17 @@ public void unregisterListener(Listener listener)
*/
public void unregisterListeners(Plugin plugin)
{
for ( Iterator<Listener> it = listenersByPlugin.get( plugin ).iterator(); it.hasNext(); )
listenersLock.lock();
try
{
for ( Iterator<Listener> it = listenersByPlugin.get( plugin ).iterator(); it.hasNext(); )
{
eventBus.unregister( it.next() );
it.remove();
}
} finally
{
eventBus.unregister( it.next() );
it.remove();
listenersLock.unlock();
}
}

Expand All @@ -475,7 +531,14 @@ public void unregisterListeners(Plugin plugin)
*/
public Collection<Map.Entry<String, Command>> getCommands()
{
return Collections.unmodifiableCollection( commandMap.entrySet() );
commandsLock.readLock().lock();
try
{
return Collections.unmodifiableCollection( commandMap.entrySet() );
} finally
{
commandsLock.readLock().unlock();
}
}

boolean isTransitiveDepend(PluginDescription plugin, PluginDescription depend)
Expand Down