ClientTicker.addAction is broken
Johni0702 opened this issue ยท 0 comments
Re-posting this as a separate issue since it seems to have been forgotten (ref).
This may or may not now be causing Johni0702/BetterPortals#395 (might be a mod re-submitting tasks in its handler or a broken ArrayDeque
due to thread-unsafe access. at least in quark there doesn't appear to be the former).
wrt 34d2080:
This seems more like a workaround than a fix.
Why is the Runnablenull
in the first place? Is that expected behavior?
It looks like to me that passingnull
toaddAction
can never serve any purpose. So havingaddAction
throw a NPE and then fixing the caller sounds like a better solution.
Actually, upon looking at Quark, I realized that this isn't ever getting called with
null
(might still want to add the check for good measure) but instead it's getting called from other threads andpendingActions
is anArrayDeque
which isn't thread-safe.
So this is definitely not the right way to fix it.
While on the topic:
Why does this even exist in the first place? I.e. why not just useMinecraft.addScheduledTask
?
As is this will additionally cause compatibility issues with BetterPortals because BP hooksMinecraft.addScheduledTask
to determine which world a particular runnable belongs to and will then make sure it's run in that context. It doesn't hook thependingActions
list so everything will just be ran in the main world.