Folia Hide/Show Player Race Condition Fix

[R00tB33rMan]

This PR introduces a few changes:

• Fixes: [1.21.8] [Folia] Floodgate causing soft-crash #610 > Explanation: A player can move thread contexts between the execution of hide/show. To mitigate this, we should ensure that moving thread contexts for this execution does not occur. This is also because you effectively shouldn't/cannot see the player in another region context.
• Resolves conditions on shutdown where the ExecutorService fails to properly shutdown/disable.
• Removes unneeded imports and resolves import order.

[Tim203]

I'm not sure how this would solve your first point. You call the on.getScheduler() on the same time for both the hide- & showPlayer. So if the player switched from executors like you said then wouldn't this version be prune to the same issue? And now you've also introduced a race condition, because you're not waiting for the result of hidePlayer before calling showPlayer. You're calling both of them (in the background) at the same time.

I've also looked at the EntityScheduler real quick (https://jd.papermc.io/folia/1.21/io/papermc/paper/threadedregions/scheduler/EntityScheduler.html) and the way it's worded I think we shouldn't have to worry about the player switching between owning regions

[R00tB33rMan]

I'm not sure how this would solve your first point. You call the on.getScheduler() on the same time for both the hide- & showPlayer. So if the player switched from executors like you said then wouldn't this version be prune to the same issue? And now you've also introduced a race condition, because you're not waiting for the result of hidePlayer before calling showPlayer. You're calling both of them (in the background) at the same time.

I've also looked at the EntityScheduler real quick (https://jd.papermc.io/folia/1.21/io/papermc/paper/threadedregions/scheduler/EntityScheduler.html) and the way it's worded I think we shouldn't have to worry about the player switching between owning regions

I was under the impression that I managed to resolve this issue, given I was experiencing this issue once/week (or more often) and since this change with a good 2 month period with identical figures, the issue was then otherwise resolved. How would you propose I approach a more adequate fix?

[R00tB33rMan]

I'm not sure how this would solve your first point. You call the on.getScheduler() on the same time for both the hide- & showPlayer. So if the player switched from executors like you said then wouldn't this version be prune to the same issue? And now you've also introduced a race condition, because you're not waiting for the result of hidePlayer before calling showPlayer. You're calling both of them (in the background) at the same time.

I've also looked at the EntityScheduler real quick (https://jd.papermc.io/folia/1.21/io/papermc/paper/threadedregions/scheduler/EntityScheduler.html) and the way it's worded I think we shouldn't have to worry about the player switching between owning regions

Hey @Tim203, are you more inclined for me to utilize this approach instead?

[Tim203]

Hey @Tim203, are you more inclined for me to utilize this approach instead?

Yes, if this'd actually solve the issue.
I think that it's quite weird though that the instance that's returned by getScheduler can change though, given the wording in the docs. I'll ask around a bit