diff --git a/api/src/main/java/net/md_5/bungee/api/plugin/Plugin.java b/api/src/main/java/net/md_5/bungee/api/plugin/Plugin.java index a62e38d19..96405b006 100644 --- a/api/src/main/java/net/md_5/bungee/api/plugin/Plugin.java +++ b/api/src/main/java/net/md_5/bungee/api/plugin/Plugin.java @@ -96,8 +96,9 @@ public class Plugin { if ( service == null ) { - service = Executors.newCachedThreadPool( new ThreadFactoryBuilder().setNameFormat( getDescription().getName() + " Pool Thread #%1$d" ) - .setThreadFactory( new GroupedThreadFactory( this ) ).build() ); + String name = ( getDescription() == null ) ? "unknown" : getDescription().getName(); + service = Executors.newCachedThreadPool( new ThreadFactoryBuilder().setNameFormat( name + " Pool Thread #%1$d" ) + .setThreadFactory( new GroupedThreadFactory( this, name ) ).build() ); } return service; } diff --git a/api/src/main/java/net/md_5/bungee/api/scheduler/GroupedThreadFactory.java b/api/src/main/java/net/md_5/bungee/api/scheduler/GroupedThreadFactory.java index 4f7ba39b2..34b19c161 100644 --- a/api/src/main/java/net/md_5/bungee/api/scheduler/GroupedThreadFactory.java +++ b/api/src/main/java/net/md_5/bungee/api/scheduler/GroupedThreadFactory.java @@ -21,9 +21,9 @@ public class GroupedThreadFactory implements ThreadFactory } - public GroupedThreadFactory(Plugin plugin) + public GroupedThreadFactory(Plugin plugin, String name) { - this.group = new BungeeGroup( plugin.getDescription().getName() ); + this.group = new BungeeGroup( name ); } @Override diff --git a/proxy/src/main/java/net/md_5/bungee/scheduler/BungeeScheduler.java b/proxy/src/main/java/net/md_5/bungee/scheduler/BungeeScheduler.java index e3085db22..1dc1cc66e 100644 --- a/proxy/src/main/java/net/md_5/bungee/scheduler/BungeeScheduler.java +++ b/proxy/src/main/java/net/md_5/bungee/scheduler/BungeeScheduler.java @@ -19,6 +19,7 @@ import net.md_5.bungee.api.scheduler.TaskScheduler; public class BungeeScheduler implements TaskScheduler { + private final Object lock = new Object(); private final AtomicInteger taskCounter = new AtomicInteger(); private final TIntObjectMap tasks = TCollections.synchronizedMap( new TIntObjectHashMap() ); private final Multimap tasksByPlugin = Multimaps.synchronizedMultimap( HashMultimap.create() ); @@ -36,9 +37,19 @@ public class BungeeScheduler implements TaskScheduler @Override public void cancel(int id) { - BungeeTask task = tasks.remove( id ); + BungeeTask task = tasks.get( id ); + Preconditions.checkArgument( task != null, "No task with id %s", id ); + task.cancel(); - tasksByPlugin.values().remove( task ); + } + + void cancel0(BungeeTask task) + { + synchronized ( lock ) + { + tasks.remove( task.getId() ); + tasksByPlugin.values().remove( task ); + } } @Override @@ -80,8 +91,13 @@ public class BungeeScheduler implements TaskScheduler Preconditions.checkNotNull( owner, "owner" ); Preconditions.checkNotNull( task, "task" ); BungeeTask prepared = new BungeeTask( this, taskCounter.getAndIncrement(), owner, task, delay, period, unit ); - tasks.put( prepared.getId(), prepared ); - tasksByPlugin.put( owner, prepared ); + + synchronized ( lock ) + { + tasks.put( prepared.getId(), prepared ); + tasksByPlugin.put( owner, prepared ); + } + owner.getExecutorService().execute( prepared ); return prepared; } diff --git a/proxy/src/main/java/net/md_5/bungee/scheduler/BungeeTask.java b/proxy/src/main/java/net/md_5/bungee/scheduler/BungeeTask.java index 195b50d44..67db0cc4e 100644 --- a/proxy/src/main/java/net/md_5/bungee/scheduler/BungeeTask.java +++ b/proxy/src/main/java/net/md_5/bungee/scheduler/BungeeTask.java @@ -38,7 +38,7 @@ public class BungeeTask implements Runnable, ScheduledTask if ( wasRunning ) { - sched.cancel( this.getId() ); + sched.cancel0( this ); } } diff --git a/proxy/src/test/java/net/md_5/bungee/api/plugin/DummyPlugin.java b/proxy/src/test/java/net/md_5/bungee/api/plugin/DummyPlugin.java new file mode 100644 index 000000000..3e2f126d2 --- /dev/null +++ b/proxy/src/test/java/net/md_5/bungee/api/plugin/DummyPlugin.java @@ -0,0 +1,11 @@ +package net.md_5.bungee.api.plugin; + +import lombok.AccessLevel; +import lombok.NoArgsConstructor; + +@NoArgsConstructor(access = AccessLevel.PRIVATE) +public class DummyPlugin extends Plugin +{ + + public static final DummyPlugin INSTANCE = new DummyPlugin(); +} diff --git a/proxy/src/test/java/net/md_5/bungee/scheduler/SchedulerTest.java b/proxy/src/test/java/net/md_5/bungee/scheduler/SchedulerTest.java new file mode 100644 index 000000000..25ac3729e --- /dev/null +++ b/proxy/src/test/java/net/md_5/bungee/scheduler/SchedulerTest.java @@ -0,0 +1,86 @@ +package net.md_5.bungee.scheduler; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import net.md_5.bungee.api.plugin.DummyPlugin; +import net.md_5.bungee.api.scheduler.ScheduledTask; +import net.md_5.bungee.api.scheduler.TaskScheduler; +import org.junit.Assert; +import org.junit.Test; + +public class SchedulerTest +{ + + @Test + public void testRun() throws InterruptedException + { + TaskScheduler scheduler = new BungeeScheduler(); + + final CountDownLatch latch = new CountDownLatch( 1 ); + + scheduler.runAsync( DummyPlugin.INSTANCE, new Runnable() + { + + @Override + public void run() + { + latch.countDown(); + } + } ); + + latch.await( 5, TimeUnit.SECONDS ); + + Assert.assertEquals( 0, latch.getCount() ); + } + + @Test + public void testCancel() throws InterruptedException + { + TaskScheduler scheduler = new BungeeScheduler(); + AtomicBoolean b = new AtomicBoolean(); + + ScheduledTask task = setup( scheduler, b ); + scheduler.cancel( task.getId() ); + Thread.sleep( 250 ); + Assert.assertFalse( b.get() ); + + task = setup( scheduler, b ); + scheduler.cancel( task ); + Thread.sleep( 250 ); + Assert.assertFalse( b.get() ); + + task = setup( scheduler, b ); + scheduler.cancel( task.getOwner() ); + Thread.sleep( 250 ); + Assert.assertFalse( b.get() ); + } + + @Test + public void testScheduleAndRepeat() throws InterruptedException + { + TaskScheduler scheduler = new BungeeScheduler(); + AtomicBoolean b = new AtomicBoolean(); + + setup( scheduler, b ); + Thread.sleep( 250 ); + Assert.assertTrue( b.get() ); + + b.set( false ); + Thread.sleep( 250 ); + Assert.assertTrue( b.get() ); + } + + private ScheduledTask setup(TaskScheduler scheduler, final AtomicBoolean hasRun) + { + return scheduler.schedule( DummyPlugin.INSTANCE, new Runnable() + { + + @Override + public void run() + { + hasRun.set( true ); + } + }, 100, 100, TimeUnit.MILLISECONDS ); + } +}