Skip to content

async pathfinding: fix path race condition#620

Merged
Dreeam-qwq merged 1 commit intoWinds-Studio:ver/1.21.11from
hayanesuru:p/42-path-race
Feb 25, 2026
Merged

async pathfinding: fix path race condition#620
Dreeam-qwq merged 1 commit intoWinds-Studio:ver/1.21.11from
hayanesuru:p/42-path-race

Conversation

@hayanesuru
Copy link
Collaborator

@hayanesuru hayanesuru commented Feb 2, 2026

#519 #589

Closed #604

@hayanesuru hayanesuru force-pushed the p/42-path-race branch 3 times, most recently from c646459 to f317d41 Compare February 8, 2026 17:32
Copy link
Collaborator

@MartijnMuijsers MartijnMuijsers left a comment

Choose a reason for hiding this comment

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

.

* ConcurrentLinkedQueue is thread-safe, non-blocking and non-synchronized
*/
private final ConcurrentLinkedQueue<Runnable> postProcessing = new ConcurrentLinkedQueue<>();
private final ArrayList<Consumer<Path>> postProcessing = new ArrayList<>();
Copy link
Collaborator

@MartijnMuijsers MartijnMuijsers Feb 20, 2026

Choose a reason for hiding this comment

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

Good change!

Copy link
Collaborator

@MartijnMuijsers MartijnMuijsers Feb 20, 2026

Choose a reason for hiding this comment

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

I'm assuming schedulePostProcessing is only called from the main thread, correct? Otherwise the .add will cause issues when called from multiple threads

if (!ready) {
Path ret = this.ret;
if (ret != null) {
complete(ret);
Copy link
Collaborator

@MartijnMuijsers MartijnMuijsers Feb 20, 2026

Choose a reason for hiding this comment

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

If this path is only completed upon isDone, what happens upon isProcessed? Shoud it also call complete?

I assume isDone() and isProcessed() can only be run from the main thread, right?

}

return this.positions.containsAll(positions);
return this.positions.equals(positions);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Original is containsAll, this changes actual behavior

@@ -67,7 +67,7 @@ protected static CompletableFuture<Void> queue(AsyncPath path) {
*/
public static void awaitProcessing(@Nullable Path path, Consumer<@Nullable Path> afterProcessing) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This already existed in the original, but the name await here is not really correct. applyAfterProcessing?

Copy link
Member

@Dreeam-qwq Dreeam-qwq left a comment

Choose a reason for hiding this comment

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

Nice PR! Thanks~

@Dreeam-qwq Dreeam-qwq merged commit 8287db8 into Winds-Studio:ver/1.21.11 Feb 25, 2026
1 check passed
@Dreeam-qwq Dreeam-qwq added the type: fix Pull request for fixing bug label Feb 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: fix Pull request for fixing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Entity threw exception (ConcurrentModificationException). Async pathfinding bug?

3 participants