110 lines
5.6 KiB
Diff
110 lines
5.6 KiB
Diff
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
From: Spottedleaf <Spottedleaf@users.noreply.github.com>
|
|
Date: Mon, 15 May 2023 11:34:28 -0700
|
|
Subject: [PATCH] Mark POI/Entity load tasks as completed before releasing
|
|
scheduling lock
|
|
|
|
It must be marked as completed during that lock hold since the
|
|
waiters field is set to null. Thus, any other thread attempting
|
|
a cancellation will fail to remove from waiters. Also, any
|
|
other thread attempting to cancel may set the completed field
|
|
to true which would cause accept() to fail as well.
|
|
|
|
Completion was always designed to happen while holding the
|
|
scheduling lock to prevent these race conditions. The code
|
|
was originally set up to complete while not holding the
|
|
scheduling lock to avoid invoking callbacks while holding the
|
|
lock, however the access to the completion field was not
|
|
considered.
|
|
|
|
Resolve this by marking the callback as completed during the
|
|
lock, but invoking the accept() function after releasing
|
|
the lock. This will prevent any cancellation attempts to be
|
|
blocked, and allow the current thread to complete the callback
|
|
without any issues.
|
|
|
|
diff --git a/src/main/java/io/papermc/paper/chunk/system/scheduling/NewChunkHolder.java b/src/main/java/io/papermc/paper/chunk/system/scheduling/NewChunkHolder.java
|
|
index 8013dd333e27aa5fd0beb431fa32491eec9f5246..efc9b7a304f10b6a23a36cffb0a4aaea6ab71129 100644
|
|
--- a/src/main/java/io/papermc/paper/chunk/system/scheduling/NewChunkHolder.java
|
|
+++ b/src/main/java/io/papermc/paper/chunk/system/scheduling/NewChunkHolder.java
|
|
@@ -156,6 +156,12 @@ public final class NewChunkHolder {
|
|
LOGGER.error("Unhandled entity data load exception, data data will be lost: ", result.right());
|
|
}
|
|
|
|
+ // Folia start - mark these tasks as completed before releasing the scheduling lock
|
|
+ for (final GenericDataLoadTaskCallback callback : waiters) {
|
|
+ callback.markCompleted();
|
|
+ }
|
|
+ // Folia end - mark these tasks as completed before releasing the scheduling lock
|
|
+
|
|
completeWaiters = waiters;
|
|
} else {
|
|
// cancelled
|
|
@@ -187,7 +193,7 @@ public final class NewChunkHolder {
|
|
// avoid holding the scheduling lock while completing
|
|
if (completeWaiters != null) {
|
|
for (final GenericDataLoadTaskCallback callback : completeWaiters) {
|
|
- callback.accept(result);
|
|
+ callback.acceptCompleted(result); // Folia - mark these tasks as completed before releasing the scheduling lock
|
|
}
|
|
}
|
|
|
|
@@ -273,6 +279,12 @@ public final class NewChunkHolder {
|
|
LOGGER.error("Unhandled poi load exception, poi data will be lost: ", result.right());
|
|
}
|
|
|
|
+ // Folia start - mark these tasks as completed before releasing the scheduling lock
|
|
+ for (final GenericDataLoadTaskCallback callback : waiters) {
|
|
+ callback.markCompleted();
|
|
+ }
|
|
+ // Folia end - mark these tasks as completed before releasing the scheduling lock
|
|
+
|
|
completeWaiters = waiters;
|
|
} else {
|
|
// cancelled
|
|
@@ -304,7 +316,7 @@ public final class NewChunkHolder {
|
|
// avoid holding the scheduling lock while completing
|
|
if (completeWaiters != null) {
|
|
for (final GenericDataLoadTaskCallback callback : completeWaiters) {
|
|
- callback.accept(result);
|
|
+ callback.acceptCompleted(result); // Folia - mark these tasks as completed before releasing the scheduling lock
|
|
}
|
|
}
|
|
this.scheduler.schedulingLock.lock();
|
|
@@ -357,7 +369,7 @@ public final class NewChunkHolder {
|
|
}
|
|
}
|
|
|
|
- public static abstract class GenericDataLoadTaskCallback implements Cancellable, Consumer<GenericDataLoadTask.TaskResult<?, Throwable>> {
|
|
+ public static abstract class GenericDataLoadTaskCallback implements Cancellable { // Folia - mark callbacks as completed before unlocking scheduling lock
|
|
|
|
protected final Consumer<GenericDataLoadTask.TaskResult<?, Throwable>> consumer;
|
|
protected final NewChunkHolder chunkHolder;
|
|
@@ -393,13 +405,23 @@ public final class NewChunkHolder {
|
|
return this.completed = true;
|
|
}
|
|
|
|
- @Override
|
|
- public void accept(final GenericDataLoadTask.TaskResult<?, Throwable> result) {
|
|
+ // Folia start - mark callbacks as completed before unlocking scheduling lock
|
|
+ // must hold scheduling lock
|
|
+ void markCompleted() {
|
|
+ if (this.completed) {
|
|
+ throw new IllegalStateException("May not be completed here");
|
|
+ }
|
|
+ this.completed = true;
|
|
+ }
|
|
+ // Folia end - mark callbacks as completed before unlocking scheduling lock
|
|
+
|
|
+ // Folia - mark callbacks as completed before unlocking scheduling lock
|
|
+ void acceptCompleted(final GenericDataLoadTask.TaskResult<?, Throwable> result) {
|
|
if (result != null) {
|
|
- if (this.setCompleted()) {
|
|
+ if (this.completed) { // Folia - mark callbacks as completed before unlocking scheduling lock
|
|
this.consumer.accept(result);
|
|
} else {
|
|
- throw new IllegalStateException("Cannot be cancelled at this point");
|
|
+ throw new IllegalStateException("Cannot be uncompleted at this point"); // Folia - mark callbacks as completed before unlocking scheduling lock
|
|
}
|
|
} else {
|
|
throw new NullPointerException("Result cannot be null (cancelled)");
|