Properly resolve rare Folia region blockages when retrieving getElevation, using managedBlock, using managedBlock

[R00tB33rMan]

This properly resolves rare conditions with getElevation which cause regions to effectively implode. Although this is a rare race condition, it is very much real and should be dealt with promptly. The previously applied fix did not adequately resolve this, since blocking with .join on Folia is simply not a good strategy.

[pop4959]

Hi, thanks for the PR!

Going to ping @Warriorrrr on here in case they wish to review since they PRed the previous fix and may have some further input on this.

Quoting from the Discussion on Discord for context

Hey, working with him here on this issue. The issue is because when you schedule the region task at that location, if its close enough to the region it's currently on(and currently blocking) then it has the potential to merge into the region thats blocking, ending to an endless block because it can't process the task it needs to process. One way to fix it is use ServerChunkCache$MainThreadExecutor#managedBlock for blocking, which would block like how you are doing rn but allow region tasks to also process, or you can remove blocking entirely in that method as blocking between 2 regions is not a great idea with Folia in the first place.

[Warriorrrr]

If this works then that'd seem great to me, I agree that having the .join is not a great idea.

Removing the blocking on chunkyborder's side is definitely possible, the BorderCheckTask class would just need to be refactored a bit.

[pop4959]

Yeah refactoring that may still be better. If it's not a super intrusive change I think it's worth considering.

[Dueris]

I disagree with the refactoring, because then we would need to update chunky boreder aswell, when this could just be a one-and-done fix that requires no updates to other repos, and fixes this for anyone else potentially hooking into this like a custom plugin. The problem isn't the usage of the method, it's the method itself, so I think its probably better to fix the source of the cause, the method, rather than fix the usage of it if that makes sense

[pop4959]

I disagree with the refactoring, because then we would need to update chunky boreder aswell, when this could just be a one-and-done fix that requires no updates to other repos, and fixes this for anyone else potentially hooking into this like a custom plugin. The problem isn't the usage of the method, it's the method itself, so I think its probably better to fix the source of the cause, the method, rather than fix the usage of it if that makes sense

That's pretty much where I stand on this as well, and if needed that refactor can still be done at a later point. This is good enough from my perspective for a fix that affects about 0.1% of servers.

I'm going to approve for now, planning to merge tomorrow or so unless there are any last minute changes or comments.

[Dueris]

Wait for merging, we found a small race condition that we want to resolve

[Dueris]

Should be all good to merge now.

[Warriorrrr]

I didn't mean to say the refactoring should happen instead of this PR, just that it could happen in the future once this is already merged, sorry for the confusion.

Should also mention that this closes pop4959/ChunkyBorder#88

[R00tB33rMan]

Hi, thanks for the PR!

Going to ping @Warriorrrr on here in case they wish to review since they PRed the previous fix and may have some further input on this.

Quoting from the Discussion on Discord for context

Hey, working with him here on this issue. The issue is because when you schedule the region task at that location, if its close enough to the region it's currently on(and currently blocking) then it has the potential to merge into the region thats blocking, ending to an endless block because it can't process the task it needs to process. One way to fix it is use ServerChunkCache$MainThreadExecutor#managedBlock for blocking, which would block like how you are doing rn but allow region tasks to also process, or you can remove blocking entirely in that method as blocking between 2 regions is not a great idea with Folia in the first place.

I've been runnin' this change for a solid week without any new exceptions/errors. Just thought I should give you the heads up!

[pop4959]

I feel confident merging this now since it has been tested for about a week with no further issues. Thanks again for the PR!