Scheduler try catch (#2037)

* try catch in SchedulerImpl to individually fail tasks

* Use exception manager

* chore: add a test

---------

Co-authored-by: mworzala <mattheworzala@gmail.com>
This commit is contained in:
Samuel 2024-03-21 17:42:24 -04:00 committed by GitHub
parent 1058d88552
commit 17fd82a5c1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 31 additions and 1 deletions

View File

@ -1,6 +1,7 @@
package net.minestom.server.timer;
import it.unimi.dsi.fastutil.ints.Int2ObjectAVLTreeMap;
import net.minestom.server.MinecraftServer;
import org.jctools.queues.MpscUnboundedArrayQueue;
import org.jetbrains.annotations.NotNull;
@ -89,7 +90,14 @@ final class SchedulerImpl implements Scheduler {
}
private void handleTask(TaskImpl task) {
final TaskSchedule schedule = task.task().get();
TaskSchedule schedule;
try {
schedule = task.task().get();
} catch (Throwable t) {
MinecraftServer.getExceptionManager().handleException(new RuntimeException("Exception in scheduled task", t));
schedule = TaskSchedule.stop();
}
if (schedule instanceof TaskScheduleImpl.DurationSchedule durationSchedule) {
final Duration duration = durationSchedule.duration();
SCHEDULER.schedule(() -> safeExecute(task), duration.toMillis(), TimeUnit.MILLISECONDS);

View File

@ -1,10 +1,12 @@
package net.minestom.server.timer;
import net.minestom.server.MinecraftServer;
import org.junit.jupiter.api.Test;
import java.time.Duration;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import static org.junit.jupiter.api.Assertions.*;
@ -139,4 +141,24 @@ public class TestScheduler {
Thread.sleep(250);
assertTrue(result.get(), "Async task didn't get executed");
}
@Test
public void exceptionTask() {
MinecraftServer.init();
Scheduler scheduler = Scheduler.newScheduler();
scheduler.scheduleNextTick(() -> {
throw new RuntimeException("Test exception");
});
// This is a bit of a weird use case. I dont want this test to depend on the order the scheduler executes in
// so this is a guess that the first one wont be before all 100 of the ones scheduled below.
// Not great, but should be fine anyway.
AtomicInteger executed = new AtomicInteger(0);
for (int i = 0; i < 100; i++) {
scheduler.scheduleNextTick(executed::incrementAndGet);
}
assertDoesNotThrow(scheduler::processTick);
assertEquals(100, executed.get());
}
}