Tablist objectives throws ArrayIndexOutOfBoundsException in `HashMap#keysToArray` when accessed in another thread
montlikadani opened this issue ยท 1 comments
Type of bug
Error in console
TabList version
Reproducable in all versions of TL from when the tablist objects has added
Software version
Reproducible in all versions of Spigot and forks from when the objectives added
Relevant plugins
No response
Console error (if applicable)
java.lang.ArrayIndexOutOfBoundsException: Index 112816 out of bounds for length 112816
at java.util.HashMap.keysToArray(HashMap.java:952) ~[?:?]
at java.util.HashMap$KeySet.toArray(HashMap.java:995) ~[?:?]
at java.util.ArrayList.<init>(ArrayList.java:181) ~[?:?]
at com.google.common.collect.Lists.newArrayList(Lists.java:132) ~[patched_1.17.1.jar:git-Paper-399]
at net.minecraft.world.scores.Scoreboard.getTrackedPlayers(Scoreboard.java:115) ~[patched_1.17.1.jar:git-Paper-399]
at org.bukkit.craftbukkit.v1_17_R1.scoreboard.CraftScore.getScore(CraftScore.java:44) ~[patched_1.17.1.jar:git-Paper-399]
at hu.montlikadani.tablist.Objects.lambda$startTask$1(Objects.java:125) ~[TabList-bukkit-5.6.0.jar:?]
TabList configuration files
Not required, reproducible with the default enabled tablist objectives.
Bug description
The exception refers to this line:
Within this is the
Objective#getScore method, in which non-thread safe changes are made to map reference. When another asynchronous thread calls the non thread-safe method(s), as in this case, and the size of the Map changes due to one or more new elements added, it throws an ArrayIndexOutOfBoundsException and this by repetition as it runs on a continuously running asynchronous thread. The source of the HashMap#keysToArray method shows that indexing is not handled properly because the value would start counting from 0 (as the initial value of the elements added to the array in C-type languages), but in this implementation it already increases indexing by +1, which will later throwing the above mentioned exception.
The HashMap#keysToArray method implementation is as follow:
L943-L955
This is equally true for the valuesToArray method, as it has only values with the same implementation.
Reproduction process/simple approach
It can also be reproduced in a simple application:
// Create a new empty map
java.util.Map<Integer, java.util.Map<String, Integer>> map = new java.util.HashMap<>();
// Perform the iteration in asynchronous thread
java.util.concurrent.CompletableFuture.supplyAsync(() -> {
// Can be any number, it is more reproducible if it more than 100+
for (int i = 0; i <= 112816; i++) {
java.util.Map<String, Integer> map2 = new java.util.HashMap<>();
map2.put("fg", 5);
map.put(i, map2);
}
return 1;
}).whenComplete((a, th) -> {
// Here the thread is done, so this will not throw ArrayIndexOutOfBoundsException
new java.util.ArrayList<>(map.keySet());
});
// ArrayIndexOutOfBoundsException will be thrown here as the "map" changed in the asynchronous thread and its not done yet
new java.util.ArrayList<>(map.keySet());Current solution
Not much can be done. For now, the solution is to switch to reflection, which is slower but more reliable (unplanned).
It is not yet reported to the JDK as it is probably just a misunderstood thread problem. On the other hand, it is not easy to report exceptions/errors found by the user to JDK (I think it needs to be confirmed by one of the group member of JDK).
A simple solution to this problem in HashMap is (I only copied two lines):
for (; e != null && idx < r.length; e = e.next) {
r[idx] = e.key;
idx++;
}