Repository navigation
Conversation
reset() and CharGetBlocks.trim() take every section lock, but readers take a section lock before sectionLock (CharBlocks EMPTY.get -> update -> getSections). internalCall, updateGet and trim(true) called them while holding the sectionLock write lock, so a reader on a neighbouring chunk could deadlock the blocking executor and stall every queued edit. Run them after the write lock is released instead, in all adapters.
There was a problem hiding this comment.
Note
Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.
🔵 Needs a closer look
This change adjusts concurrency and lock ordering across multiple version-specific Paperweight NMS adapters, making it high-impact and safety-critical enough that it needs live runtime sanity validation on supported Paper server versions to rule out new stalls, races, or regressions.
No open issues 🎉
What changed in this PR
This pull request updates the Bukkit Paperweight PaperweightGetBlocks implementations to avoid a lock-order inversion that could deadlock the blocking executor (and stall queued edits) by ensuring operations that take all section locks are not executed while holding the chunk cache sectionLock write lock.
Changes:
- Defers full-cache
reset()calls ininternalCall(...)until after releasingsectionLock. - Defers full-cache
reset()calls inupdateGet(...)until after releasingsectionLock. - Moves aggressive
trim(true)/super.trim(true)invocation to run after releasingsectionLock(across all affected adapters).
Assessment: This pull request is too complex or risky for automated approval.
| File | Summary |
|---|---|
worldedit-bukkit/adapters/adapter-26.3/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v26_3/PaperweightGetBlocks.java |
Defers full resets/aggressive trims to run outside sectionLock to prevent lock-order deadlocks. |
worldedit-bukkit/adapters/adapter-26.2/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v26_2/PaperweightGetBlocks.java |
Applies the same lock-order deadlock avoidance by moving full reset/trim work outside sectionLock. |
worldedit-bukkit/adapters/adapter-26.1/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v26_1/PaperweightGetBlocks.java |
Applies the same lock-order deadlock avoidance by moving full reset/trim work outside sectionLock. |
worldedit-bukkit/adapters/adapter-1_21/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_R1/PaperweightGetBlocks.java |
Applies the same lock-order deadlock avoidance by moving full reset/trim work outside sectionLock. |
worldedit-bukkit/adapters/adapter-1_21_9/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_9/PaperweightGetBlocks.java |
Applies the same lock-order deadlock avoidance by moving full reset/trim work outside sectionLock. |
worldedit-bukkit/adapters/adapter-1_21_6/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_6/PaperweightGetBlocks.java |
Applies the same lock-order deadlock avoidance by moving full reset/trim work outside sectionLock. |
worldedit-bukkit/adapters/adapter-1_21_5/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_5/PaperweightGetBlocks.java |
Applies the same lock-order deadlock avoidance by moving full reset/trim work outside sectionLock. |
worldedit-bukkit/adapters/adapter-1_21_4/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_4/PaperweightGetBlocks.java |
Applies the same lock-order deadlock avoidance by moving full reset/trim work outside sectionLock. |
worldedit-bukkit/adapters/adapter-1_21_11/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_11/PaperweightGetBlocks.java |
Applies the same lock-order deadlock avoidance by moving full reset/trim work outside sectionLock. |
Review effort: Balanced
|
Is this not effectively fixed by #3543 ? |
@dordsor21 Not from what I was told. I had the user run that version on their server and they said it didn't work but it's possible they gave up on that build and changed to stable or a different snapshot. |
|
I think you're probably right, but it would be nice to see the thread dump of the issue as well |
|
@dordsor21 I'll see if they still have a relevant one. |
reset()andCharGetBlocks.trim()take every section lock, but readers take a section lock beforesectionLock(CharBlocks EMPTY.get -> update -> getSections).internalCall,updateGetandtrim(true)called them while holding thesectionLockwrite lock, so a reader on a neighbouring chunk could deadlock the blocking executor and stall every queued edit. Run them after the write lock is released instead, in all adapters.This change has been stress tested for a week on an active server where the issue it fixes occurred.