search

WasabiThumb / xclaim

A better chunk claiming system for Paper servers
Mozilla Public License 2.0
19 stars 10 forks source link

Server crashes when some player tries to claim another persons claim #122

Closed Krayir5 closed 2 months ago

Krayir5 commented 2 months ago

Firstly, some player in server tried to claim another persons claim and then server crashed.Then I tried to find a setting to stop other players to claim anothers persons claim but i couldn't find it anywhere and anytime someone tries this server crashes

WasabiThumb commented 2 months ago

What is the error that causes the server crash? Can you send the log?

Krayir5 commented 2 months ago

message.txt Here is the log

Krayir5 commented 2 months ago

The reason that is crashing is probably is when a person tries to claim a chunk plugin tries to remove the claim to claim to him but he doesn't have a permission to unclaim and then plugin crashes if you look to that i will be grateful.

WasabiThumb commented 2 months ago

The reason that is crashing is probably is when a person tries to claim a chunk plugin tries to remove the claim to claim to him but he doesn't have a permission to unclaim and then plugin crashes if you look to that i will be grateful.

That is not the reason for the crash, there's a deadlock happening as the plugin is never able to acquire the registry write lock. Sorry for the lack of response, I just haven't had a chance to look at it yet.

Krayir5 commented 2 months ago

The reason that is crashing is probably is when a person tries to claim a chunk plugin tries to remove the claim to claim to him but he doesn't have a permission to unclaim and then plugin crashes if you look to that i will be grateful.

That is not the reason for the crash, there's a deadlock happening as the plugin is never able to acquire the registry write lock. Sorry for the lack of response, I just haven't had a chance to look at it yet.

So how can i fix the deadlock can you help?

WasabiThumb commented 2 months ago

The reason that is crashing is probably is when a person tries to claim a chunk plugin tries to remove the claim to claim to him but he doesn't have a permission to unclaim and then plugin crashes if you look to that i will be grateful.

That is not the reason for the crash, there's a deadlock happening as the plugin is never able to acquire the registry write lock. Sorry for the lack of response, I just haven't had a chance to look at it yet.

So how can i fix the deadlock can you help?

Now that I've had a chance to look at the code, here's a more detailed step-by-step description of the issue:

  1. Despite the fact that the claiming player does not own the chunk that they are overriding, the code to catch this case does not run. This will happen if the player has the permission xclaim.override or is an OP, so as a bandaid fix, you can make sure that your players do not have this permission. But moving on.
  2. We call Claim#addChunk which claims the chunk unconditionally. If we get to this point, we're assuming that all permission checks have passed. This means that addChunk has the responsibility to also remove that chunk from any claims that it is currently a part of.
  3. addChunk adds the chunk to the claim's chunk list.
  4. addChunk acquires a read lock on the registry, to make sure that no claims can be deleted while it checks if another claim also holds this chunk.
  5. addChunk finds a claim that holds the same chunk, and calls removeChunk on it.
  6. removeChunk successfully removes the chunk from the conflicting claim. However, since the conflicting claim now holds 0 chunks, it must be deleted. Therefore unclaim is called.
  7. unclaim attempts to acquire a write lock on the registry in order to remove the current claim from it, but ends up waiting forever (deadlock) because the read lock from step 4 is still held.
  8. The server eventually notices that it's hung, produces a thread dump and shuts down.

TLDR; this is an edge case that I never caught which happens only when a player who has override permissions steals a chunk from an existing claim, causing the existing claim to delete itself due to being empty. To fix this, I basically just have to unlock the registry somewhere between step 4 and 7, wherever turns out to be best API-wise. I will sneak this into release 1.14, which you can track here.

WasabiThumb commented 2 months ago

Here is the bleeding-edge dev build from the CI if you really want it (find artifact, click the download button, unzip). There is at least 1 change I want to make before public release.

Krayir5 commented 2 months ago

It's fixed thanks! It works as fine as a diamond mate.