Fix concurrency issue in light engine (Vanilla bug)

Mojang implemented a cache like chunks have, but this cache
is accessed by multiple threads and is totally not safe.

So just remove it

Fixes #3466

Also missed a pooled nibble release, so slid that in there too.
This commit is contained in:
Aikar 2020-05-28 16:59:38 -04:00
parent 0d5ec65372
commit 919ab67a13
No known key found for this signature in database
GPG Key ID: 401ADFC9891FAAFE
2 changed files with 40 additions and 8 deletions

View File

@ -63,7 +63,7 @@ index a3f919816eb2a742ed09b553995e6508684e5ea5..88277d23c36696fdd5363e41a130c9a4
} }
diff --git a/src/main/java/net/minecraft/server/LightEngineStorageArray.java b/src/main/java/net/minecraft/server/LightEngineStorageArray.java diff --git a/src/main/java/net/minecraft/server/LightEngineStorageArray.java b/src/main/java/net/minecraft/server/LightEngineStorageArray.java
index b978723a66d001f70325df0c7521025e079d7cfa..278aec8846d3bd448e359095063a711e78213ee5 100644 index b978723a66d001f70325df0c7521025e079d7cfa..fe35e297d9283d2826e8985806ee268ac2020bfb 100644
--- a/src/main/java/net/minecraft/server/LightEngineStorageArray.java --- a/src/main/java/net/minecraft/server/LightEngineStorageArray.java
+++ b/src/main/java/net/minecraft/server/LightEngineStorageArray.java +++ b/src/main/java/net/minecraft/server/LightEngineStorageArray.java
@@ -8,10 +8,17 @@ public abstract class LightEngineStorageArray<M extends LightEngineStorageArray< @@ -8,10 +8,17 @@ public abstract class LightEngineStorageArray<M extends LightEngineStorageArray<
@ -87,7 +87,7 @@ index b978723a66d001f70325df0c7521025e079d7cfa..278aec8846d3bd448e359095063a711e
this.c(); this.c();
this.d = true; this.d = true;
} }
@@ -19,12 +26,13 @@ public abstract class LightEngineStorageArray<M extends LightEngineStorageArray< @@ -19,26 +26,33 @@ public abstract class LightEngineStorageArray<M extends LightEngineStorageArray<
public abstract M b(); public abstract M b();
public void a(long i) { public void a(long i) {
@ -103,16 +103,36 @@ index b978723a66d001f70325df0c7521025e079d7cfa..278aec8846d3bd448e359095063a711e
} }
@Nullable @Nullable
@@ -37,7 +45,7 @@ public abstract class LightEngineStorageArray<M extends LightEngineStorageArray< public NibbleArray c(long i) {
+ // Paper start - remove cache - not thread safe
+ /*
if (this.d) {
for (int j = 0; j < 2; ++j) {
if (i == this.b[j]) {
return this.c[j];
} }
} }
- }
+ }*/
+ // Paper end
- NibbleArray nibblearray = (NibbleArray) this.a.get(i); - NibbleArray nibblearray = (NibbleArray) this.a.get(i);
+ NibbleArray nibblearray = (NibbleArray) (this.isVisible ? this.data.getVisibleAsync(i) : this.data.getUpdating(i)); // Paper - avoid copying light data + NibbleArray nibblearray = (NibbleArray) (this.isVisible ? this.data.getVisibleAsync(i) : this.data.getUpdating(i)); // Paper - avoid copying light data
+ // Paper start - remove cache - not thread safe
+ return nibblearray;
+ /*
if (nibblearray == null) { if (nibblearray == null) {
return null; return null;
@@ -58,11 +66,13 @@ public abstract class LightEngineStorageArray<M extends LightEngineStorageArray< } else {
@@ -53,24 +67,28 @@ public abstract class LightEngineStorageArray<M extends LightEngineStorageArray<
}
return nibblearray;
- }
+ }*/
+ // Paper end
}
@Nullable @Nullable
public NibbleArray d(long i) { public NibbleArray d(long i) {
@ -128,6 +148,16 @@ index b978723a66d001f70325df0c7521025e079d7cfa..278aec8846d3bd448e359095063a711e
} }
public void c() { public void c() {
+ /* // Paper - remove cache
for (int i = 0; i < 2; ++i) {
this.b[i] = Long.MAX_VALUE;
this.c[i] = null;
}
-
+ */
}
public void d() {
diff --git a/src/main/java/net/minecraft/server/LightEngineStorageBlock.java b/src/main/java/net/minecraft/server/LightEngineStorageBlock.java diff --git a/src/main/java/net/minecraft/server/LightEngineStorageBlock.java b/src/main/java/net/minecraft/server/LightEngineStorageBlock.java
index 0f7f4744d9f9e5174fb38ccc72248397a4b064ea..292d8c742d3be41ba8ad7fb7f1251dc7f790b62b 100644 index 0f7f4744d9f9e5174fb38ccc72248397a4b064ea..292d8c742d3be41ba8ad7fb7f1251dc7f790b62b 100644
--- a/src/main/java/net/minecraft/server/LightEngineStorageBlock.java --- a/src/main/java/net/minecraft/server/LightEngineStorageBlock.java

View File

@ -49,15 +49,17 @@ index 88277d23c36696fdd5363e41a130c9a443fac2c0..fa8039d38d5b3110fd85abf850248ba7
} }
diff --git a/src/main/java/net/minecraft/server/LightEngineStorageArray.java b/src/main/java/net/minecraft/server/LightEngineStorageArray.java diff --git a/src/main/java/net/minecraft/server/LightEngineStorageArray.java b/src/main/java/net/minecraft/server/LightEngineStorageArray.java
index 278aec8846d3bd448e359095063a711e78213ee5..f17b16d5c52cd77dd53807222dff4631d185e159 100644 index fe35e297d9283d2826e8985806ee268ac2020bfb..cbeb6387e1e05961d2b747bbb05d3f577f82399c 100644
--- a/src/main/java/net/minecraft/server/LightEngineStorageArray.java --- a/src/main/java/net/minecraft/server/LightEngineStorageArray.java
+++ b/src/main/java/net/minecraft/server/LightEngineStorageArray.java +++ b/src/main/java/net/minecraft/server/LightEngineStorageArray.java
@@ -27,7 +27,7 @@ public abstract class LightEngineStorageArray<M extends LightEngineStorageArray< @@ -27,7 +27,9 @@ public abstract class LightEngineStorageArray<M extends LightEngineStorageArray<
public void a(long i) { public void a(long i) {
if (this.isVisible) { throw new IllegalStateException("writing to visible data"); } // Paper - avoid copying light data if (this.isVisible) { throw new IllegalStateException("writing to visible data"); } // Paper - avoid copying light data
- this.data.queueUpdate(i, ((NibbleArray) this.data.getUpdating(i)).b()); // Paper - avoid copying light data - this.data.queueUpdate(i, ((NibbleArray) this.data.getUpdating(i)).b()); // Paper - avoid copying light data
+ this.data.queueUpdate(i, new NibbleArray().markPoolSafe(this.data.getUpdating(i).getCloneIfSet())); // Paper - avoid copying light data - pool safe clone + NibbleArray updating = this.data.getUpdating(i); // Paper - pool nibbles
+ this.data.queueUpdate(i, new NibbleArray().markPoolSafe(updating.getCloneIfSet())); // Paper - avoid copying light data - pool safe clone
+ if (updating.cleaner != null) updating.cleaner.run(); // Paper
this.c(); this.c();
} }