fix: alter synchronisation of getblocks trim implementation

[dordsor21]

thoughts?

[Copilot]

Pull Request Overview

This PR refactors synchronization in the trim method implementation across multiple version adapters to improve thread safety and performance. The change removes method-level synchronization in favor of more granular locking patterns.

• Removes synchronized modifier from trim method signatures across all version adapters
• Implements granular synchronization with proper try-finally blocks for write locks
• Updates ChunkCache to delegate synchronization responsibility to individual implementations

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
ChunkCache.java	Removes synchronized block around igb.trim() call, delegating sync to implementation
PaperweightGetBlocks.java (v1_21_6)	Refactors trim method with granular synchronization and proper lock handling
PaperweightGetBlocks.java (v1_21_5)	Identical synchronization refactoring as v1_21_6
PaperweightGetBlocks.java (v1_21_4)	Identical synchronization refactoring as v1_21_6
PaperweightGetBlocks.java (v1_21)	Identical synchronization refactoring as v1_21_6
PaperweightGetBlocks.java (v1_20_5)	Identical synchronization refactoring as v1_21_6
PaperweightGetBlocks.java (v1_20_4)	Identical synchronization refactoring as v1_21_6
PaperweightGetBlocks.java (v1_20_2)	Identical synchronization refactoring as v1_21_6

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[NotMyFault]

Looks reasonable

[SirYwell]

Do you remember what the exact deadlock situation was?

Reading levelChunk and the sections array without an appropriate lock sounds a bit dangerous, though I'm not fully sure about the details.

[dordsor21]

Do you remember what the exact deadlock situation was?

Reading levelChunk and the sections array without an appropriate lock sounds a bit dangerous, though I'm not fully sure about the details.

Deadlock was reported by a user in august specifically on trimming. It seems to be a relatively rare circumstance that can lead to the specific trim settings which is why it's not been reported till now, but looking at the old code it was possible for a deadlock.

Is there a specific place you think we should be locking? I believe the methods called from here lock where it would be suitable to do so

[SirYwell]

I don't have anything specific in mind, but I'd like to better understand the situation in which the deadlock was able to occur. Other than that, it's generally just rather difficult to keep track which lock is acquired where.

[dordsor21]

I don't have anything specific in mind, but I'd like to better understand the situation in which the deadlock was able to occur. Other than that, it's generally just rather difficult to keep track which lock is acquired where.

Basically, trim synchronised on the instance and then locked the sectionLock. internallCall did it the opposite way round (it locked the sectionLock before calling getChunk which synchronises on the instance if this.levelChunk is null (which it will be when we're trimming a lot))

[SirYwell]

Basically, trim synchronised on the instance and then locked the sectionLock. internallCall did it the opposite way round (it locked the sectionLock before calling getChunk which synchronises on the instance if this.levelChunk is null (which it will be when we're trimming a lot))

Thanks.