From 3440a1f2ee543e24224e59356d6eb6f9f58316a3 Mon Sep 17 00:00:00 2001 From: Wesley Wolfe Date: Wed, 29 Feb 2012 08:29:22 -0600 Subject: [Bleeding] Fixes deadlock issue and generic usage. Fixes BUKKIT-844 and fixes BUKKIT-854 --- .../bukkit/event/server/ServiceRegisterEvent.java | 5 +- .../event/server/ServiceUnregisterEvent.java | 5 +- .../org/bukkit/plugin/SimpleServicesManager.java | 87 +++++++++++++--------- 3 files changed, 60 insertions(+), 37 deletions(-) (limited to 'src') diff --git a/src/main/java/org/bukkit/event/server/ServiceRegisterEvent.java b/src/main/java/org/bukkit/event/server/ServiceRegisterEvent.java index 89c852d5..628110aa 100644 --- a/src/main/java/org/bukkit/event/server/ServiceRegisterEvent.java +++ b/src/main/java/org/bukkit/event/server/ServiceRegisterEvent.java @@ -4,7 +4,9 @@ import org.bukkit.event.HandlerList; import org.bukkit.plugin.RegisteredServiceProvider; /** - * This event is called when a service is registered. + * This event is called when a service is registered.
+ * Warning: The order in which register and unregister + * events are called should not be relied upon. */ public class ServiceRegisterEvent extends ServiceEvent { private static final HandlerList handlers = new HandlerList(); @@ -13,6 +15,7 @@ public class ServiceRegisterEvent extends ServiceEvent { super(registeredProvider); } + @Override public HandlerList getHandlers() { return handlers; } diff --git a/src/main/java/org/bukkit/event/server/ServiceUnregisterEvent.java b/src/main/java/org/bukkit/event/server/ServiceUnregisterEvent.java index 608f5576..e018f8e6 100644 --- a/src/main/java/org/bukkit/event/server/ServiceUnregisterEvent.java +++ b/src/main/java/org/bukkit/event/server/ServiceUnregisterEvent.java @@ -4,7 +4,9 @@ import org.bukkit.event.HandlerList; import org.bukkit.plugin.RegisteredServiceProvider; /** - * This event is called when a service is unregistered. + * This event is called when a service is unregistered.
+ * Warning: The order in which register and unregister + * events are called should not be relied upon. */ public class ServiceUnregisterEvent extends ServiceEvent { private static final HandlerList handlers = new HandlerList(); @@ -13,6 +15,7 @@ public class ServiceUnregisterEvent extends ServiceEvent { super(serviceProvider); } + @Override public HandlerList getHandlers() { return handlers; } diff --git a/src/main/java/org/bukkit/plugin/SimpleServicesManager.java b/src/main/java/org/bukkit/plugin/SimpleServicesManager.java index 94600518..4e177112 100644 --- a/src/main/java/org/bukkit/plugin/SimpleServicesManager.java +++ b/src/main/java/org/bukkit/plugin/SimpleServicesManager.java @@ -4,14 +4,17 @@ import org.bukkit.Bukkit; import org.bukkit.event.server.ServiceRegisterEvent; import org.bukkit.event.server.ServiceUnregisterEvent; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; + import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.Map; import java.util.List; import java.util.NoSuchElementException; +import java.util.Set; /** * A simple services manager. @@ -33,24 +36,26 @@ public class SimpleServicesManager implements ServicesManager { * @param priority priority of the provider */ public void register(Class service, T provider, Plugin plugin, ServicePriority priority) { - + RegisteredServiceProvider registeredProvider = null; synchronized (providers) { List> registered = providers.get(service); - if (registered == null) { registered = new ArrayList>(); providers.put(service, registered); } - RegisteredServiceProvider registeredProvider = new RegisteredServiceProvider(service, provider, priority, plugin); - registered.add(registeredProvider); + registeredProvider = new RegisteredServiceProvider(service, provider, priority, plugin); - // Make sure that providers are in the right order in order - // for priorities to work correctly - Collections.sort(registered); + // Insert the provider into the collection, much more efficient big O than sort + int position = Collections.binarySearch(registered, registeredProvider); + if (position < 0) { + registered.add(-(position + 1), registeredProvider); + } else { + registered.add(position, registeredProvider); + } - Bukkit.getServer().getPluginManager().callEvent(new ServiceRegisterEvent(registeredProvider)); } + Bukkit.getServer().getPluginManager().callEvent(new ServiceRegisterEvent(registeredProvider)); } /** @@ -59,6 +64,7 @@ public class SimpleServicesManager implements ServicesManager { * @param plugin The plugin */ public void unregisterAll(Plugin plugin) { + ArrayList unregisteredEvents = new ArrayList(); synchronized (providers) { Iterator, List>>> it = providers.entrySet().iterator(); @@ -73,9 +79,9 @@ public class SimpleServicesManager implements ServicesManager { while (it2.hasNext()) { RegisteredServiceProvider registered = it2.next(); - if (registered.getPlugin() == plugin) { + if (registered.getPlugin().equals(plugin)) { it2.remove(); - Bukkit.getServer().getPluginManager().callEvent(new ServiceUnregisterEvent(registered)); + unregisteredEvents.add(new ServiceUnregisterEvent(registered)); } } } catch (NoSuchElementException e) { // Why does Java suck @@ -88,6 +94,9 @@ public class SimpleServicesManager implements ServicesManager { } } catch (NoSuchElementException e) {} } + for (ServiceUnregisterEvent event : unregisteredEvents) { + Bukkit.getServer().getPluginManager().callEvent(event); + } } /** @@ -97,6 +106,7 @@ public class SimpleServicesManager implements ServicesManager { * @param provider The service provider implementation */ public void unregister(Class service, Object provider) { + ArrayList unregisteredEvents = new ArrayList(); synchronized (providers) { Iterator, List>>> it = providers.entrySet().iterator(); @@ -119,7 +129,7 @@ public class SimpleServicesManager implements ServicesManager { if (registered.getProvider() == provider) { it2.remove(); - Bukkit.getServer().getPluginManager().callEvent(new ServiceUnregisterEvent(registered)); + unregisteredEvents.add(new ServiceUnregisterEvent(registered)); } } } catch (NoSuchElementException e) { // Why does Java suck @@ -132,6 +142,9 @@ public class SimpleServicesManager implements ServicesManager { } } catch (NoSuchElementException e) {} } + for (ServiceUnregisterEvent event : unregisteredEvents) { + Bukkit.getServer().getPluginManager().callEvent(event); + } } /** @@ -140,6 +153,7 @@ public class SimpleServicesManager implements ServicesManager { * @param provider The service provider implementation */ public void unregister(Object provider) { + ArrayList unregisteredEvents = new ArrayList(); synchronized (providers) { Iterator, List>>> it = providers.entrySet().iterator(); @@ -154,9 +168,9 @@ public class SimpleServicesManager implements ServicesManager { while (it2.hasNext()) { RegisteredServiceProvider registered = it2.next(); - if (registered.getProvider() == provider) { + if (registered.getProvider().equals(provider)) { it2.remove(); - Bukkit.getServer().getPluginManager().callEvent(new ServiceUnregisterEvent(registered)); + unregisteredEvents.add(new ServiceUnregisterEvent(registered)); } } } catch (NoSuchElementException e) { // Why does Java suck @@ -169,6 +183,9 @@ public class SimpleServicesManager implements ServicesManager { } } catch (NoSuchElementException e) {} } + for (ServiceUnregisterEvent event : unregisteredEvents) { + Bukkit.getServer().getPluginManager().callEvent(event); + } } /** @@ -179,7 +196,6 @@ public class SimpleServicesManager implements ServicesManager { * @param service The service interface * @return provider or null */ - @SuppressWarnings("unchecked") public T load(Class service) { synchronized (providers) { List> registered = providers.get(service); @@ -189,7 +205,7 @@ public class SimpleServicesManager implements ServicesManager { } // This should not be null! - return (T) registered.get(0).getProvider(); + return service.cast(registered.get(0).getProvider()); } } @@ -222,68 +238,69 @@ public class SimpleServicesManager implements ServicesManager { * @return provider registration or null */ public List> getRegistrations(Plugin plugin) { + ImmutableList.Builder> ret = ImmutableList.>builder(); synchronized (providers) { - List> ret = new ArrayList>(); - for (List> registered : providers.values()) { for (RegisteredServiceProvider provider : registered) { - if (provider.getPlugin() == plugin) { + if (provider.getPlugin().equals(plugin)) { ret.add(provider); } } } - - return ret; } + return ret.build(); } /** * Get registrations of providers for a service. The returned list is - * unmodifiable. + * an unmodifiable copy. * * @param The service interface * @param service The service interface - * @return list of registrations + * @return a copy of the list of registrations */ @SuppressWarnings("unchecked") - public Collection> getRegistrations(Class service) { + public List> getRegistrations(Class service) { + ImmutableList.Builder> ret; synchronized (providers) { List> registered = providers.get(service); if (registered == null) { - return Collections.unmodifiableList(new ArrayList>()); + return ImmutableList.>of(); } - List> ret = new ArrayList>(); + ret = ImmutableList.>builder(); for (RegisteredServiceProvider provider : registered) { ret.add((RegisteredServiceProvider) provider); } - return Collections.unmodifiableList(ret); } + return ret.build(); } /** * Get a list of known services. A service is known if it has registered * providers for it. * - * @return list of known services + * @return a copy of the set of known services */ - public Collection> getKnownServices() { - return Collections.unmodifiableSet(providers.keySet()); + public Set> getKnownServices() { + synchronized (providers) { + return ImmutableSet.>copyOf(providers.keySet()); + } } /** - * Returns whether a provider has been registered for a service. Do not - * check this first only to call load(service) later, as that - * would be a non-thread safe situation. + * Returns whether a provider has been registered for a service. * * @param service * @param service service to check - * @return whether there has been a registered provider + * @return true if and only if there are registered providers */ public boolean isProvidedFor(Class service) { - return getRegistration(service) != null; + synchronized (providers) { + return providers.containsKey(service); + } } } -- cgit v1.2.3