Skip to content

fix: park contended area locks without losing wakeups - #13

Merged
Spottedleaf merged 1 commit into
Tuinity:masterfrom
PluginsKers:fix/area-lock-contention
Oct 20, 2026
Merged

Spottedleaf merged 1 commit into
Tuinity:masterfrom
PluginsKers:fix/area-lock-contention

Conversation

@PluginsKers

@PluginsKers PluginsKers commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

The multi-section ReentrantAreaLock.lock(fromX, fromY, toX, toY) overload rolls back a conflicting acquisition and immediately retries, repeatedly updating the concurrent hash table while the owner is still holding the area. Wait on the conflicting node's notifier after rolling back the partial acquisition, matching the single-section path.

Permanently notify and retire each partially acquired node before retrying. A contender can observe that node before rollback and start waiting afterwards; resetting its notifier with notifyThreadsAndBlock() can miss the notification and leave overlapping acquisitions waiting indefinitely. A fresh node keeps notifications associated with their original acquisition attempt.

This path appeared during a Folia profile using concurrentutil 1.2.4: one roughly 862 ms stall had 22 region threads waiting in ThreadedTicketLevelPropagator$UpdateQueue.await, while 435 of 438 execution samples from the other 22 region threads were in ReentrantAreaLock.lock and the concurrent table's putIfAbsent path. The same retry loop is present on master.

@PluginsKers
PluginsKers force-pushed the fix/area-lock-contention branch from bc9d644 to 0528d66 Compare October 19, 2026 05:36

@R00tB33rMan R00tB33rMan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm that this patch does in fact resolve what it claims. Tried against squaremap, where it derives from

R00tB33rMan added a commit to MagnoliaLLC/Canvas that referenced this pull request Oct 19, 2026
concurrentutil's ReentrantAreaLock stopped waiting in the multi-section lock when it moved to Notifier, so a thread blocked on an area spins until the area frees. A virtual thread holding a section can park, and once spinners hold every carrier it never resumes: the region threads and chunk workers wanting the same sections stall for good. Wait on the holder's notifier after a failed grab, and retire a rolled-back node instead of reusing it, since a contender may start waiting on it after the notification. From Tuinity/LeafPile#13 by PluginsKers; drop this once a LeafPile release carries it.
@PluginsKers

Copy link
Copy Markdown
Contributor Author

Can confirm that this patch does in fact resolve what it claims. Tried against squaremap, where it derives from

Thanks for confirming the fix with squaremap

Euphillya added a commit to Fidorial/Fidorial that referenced this pull request Oct 20, 2026
@Spottedleaf
Spottedleaf merged commit 0528d66 into Tuinity:master Oct 20, 2026
@Spottedleaf

Copy link
Copy Markdown
Member

0528d66

I noticed that another issue with the acquire logic is that the break logic was incorrectly done, it incorrectly only broke out of the coordinate x loop - allowing it to acquire more nodes even when contended.

I also don't think that the node needs to be created again, the node can be re-used by simply re-blocking it before acquiring more nodes (after waiting for a notification from the contended node).

b9a6efe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants