[bobhenl]

Overview

No linked issue.

Description

Request.requests is a static map keyed by thread. Actor tasks queued through AsyncNotifyKeyedQueue can set a request (edit session, extent, world) and never reset it, so the entry stays after the task ends. The edit session holds its actor, so the last player who used that pool thread stays in memory after logging out. A heap dump after ~14 h showed 4 disconnected players held only through this map.

• AsyncNotifyKeyedQueue#call now clears the thread's request in a finally block, also when the task throws.
• It only clears it if the thread had no request when the task started, so a nested task does not remove the outer task's request.
• Added Request.hasRequest() so the check does not create an empty request.

Not tested on a server yet.

Submitter Checklist

• Make sure you are opening from a topic branch (/feature/fix/docs/ branch (right side)) and not your main branch.
• Ensure that the pull request title represents the desired changelog entry.
• New public fields and methods are annotated with @since TODO.
• I read and followed the contribution guidelines.

[Copilot]

🔵 Needs a closer look

The change alters thread-local Request lifecycle on reused pool threads and is untested on a live server, so its nested-task runtime behavior needs human validation beyond automated review.

No open issues 🎉

What changed in this PR

This PR fixes a memory leak in FastAsyncWorldEdit where disconnected players were being retained in memory. Request.requests is a static ConcurrentHashMap keyed by Thread. Tasks queued through AsyncNotifyKeyedQueue run on reused ForkJoinPool worker threads and may populate a thread-local Request (holding an EditSession, which references the Actor) without ever clearing it. The stale entry then pins the last player who used that pool thread until the thread runs another task — a heap dump after ~14h showed 4 disconnected players retained solely through this map. The fix resets the request in a finally block after each top-level task, matching the populate-then-reset()-in-finally convention already used in command/tool code paths.

Changes:

• AsyncNotifyKeyedQueue#call now clears the thread's Request in a finally block (including when the task throws), guarded so only the task that created the request clears it (nested tasks don't remove the outer task's request).
• Added Request.hasRequest() to check for an existing thread request without creating an empty one via computeIfAbsent.

File	Description
worldedit-core/​.../​session/​request/​Request.java	Adds hasRequest() to non-destructively check whether the current thread already has a request.
worldedit-core/​.../​core/​util/​task/​AsyncNotifyKeyedQueue.java	Resets the thread-local Request in a finally after a top-level queued task completes, preventing EditSession/Actor retention on reused pool threads.

No blocking issues were found. The nested-task guard (ownsRequest = !Request.hasRequest()) correctly preserves an outer task's request when a nested task executes inline on the same worker thread, and the finally guarantees cleanup on both normal completion and exceptions. Since the operation is keyed by Thread.currentThread(), the non-atomic hasRequest()/reset() sequence is safe (only the owning thread mutates its own entry). This change also improves Actor#cancel() accuracy by no longer leaving completed edit sessions in Request.getAll().

_{Review effort: Balanced}

[MattBDev]

I see no problems with this change from the diff. Additionally, the PR description and the details from Copilot show it to be as straightforward as it looks while solving a legit issue that I've also noticed but haven't been bothered to fix yet. I suggest @dordsor21 and @SirYwell both approve we merge though. 😉

[SirYwell]

I'm not sure if this is the right place to do that. I'm also not sure what the semantics of a "nested task" is. Do we remove the request properly in other cases? Could you explain what's going on there?

[bobhenl]

Pool threads in AsyncNotifyKeyedQueue are reused, and Request lives in a per-thread map. If a queued task creates a Request and never resets it, that Request keeps its EditSession and the actor reachable until the same thread happens to run another task. We saw that as retained memory on our server. By “nested task” I mean a task that runs inside another task on the same thread, for example when an outer operation calls into the queue and the work runs inline. In that case the outer task already owns the Request, so the inner one must not reset it. That’s why it checks Request.hasRequest() first and only resets when the task created the Request itself. Other paths (command handling via Request.reset() in the platform command manager) already clean up after themselves; this queue was the one place we found where nothing did. It can be moved elsewhere if you prefer a different spot.