Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 731197c294
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| CompletableFuture<Void> drained = new CompletableFuture<>(); | ||
| synchronized (this) { | ||
| this.closed = true; | ||
| this.strand.execute(() -> drained.complete(null)); |
There was a problem hiding this comment.
Keep queue shutdown independent of the shared file pool
When all four Workers.files() threads are occupied by stalled reads or debugger-analysis tasks, this drain marker remains queued on that same pool while close() blocks in drained.get(). A project switch or application shutdown can therefore hang indefinitely even when this project's write queue is empty; the former dedicated write executor did not depend on unrelated file work making progress. The close path needs a way to drain independently of shared-pool availability.
Useful? React with 👍 / 👎.
d5e7629 to
295b846
Compare
731197c to
c7926a4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7926a4e4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
| }).whenComplete((ignored, failure) -> SwingUtilities.invokeLater(() -> { | ||
| }, Workers.files()).whenComplete((ignored, failure) -> SwingUtilities.invokeLater(() -> { |
There was a problem hiding this comment.
Avoid joining script saves on their own bounded pool
When three file workers are stalled and an affected script has another autosave chained behind an in-flight save, this command can take the fourth worker before that chained save is submitted, then block at pendingSave().join() while the save is queued back to Workers.files(). With every worker occupied, rename, move, or delete never completes even after the preceding save finishes; compose the operation after the pending saves instead of blocking a worker from the same bounded pool.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
295b846 to
c827f12
Compare
c7926a4 to
9c9c5db
Compare
c827f12 to
d4e9891
Compare
…he write queue WriteQueue, a serial path over the file work, is the project's write queue: the project makes it for its pipeline and closes it before the change record, and ConfigChanges keeps no thread. The async calls that ran on the JVM's shared pool run on Workers.files(); the thumbnail and logo readers use file strands, the resource viewer the file work, and the state writer's flush the timer. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
9c9c5db to
e5e3da3
Compare
Eighth PR of
docs/SYSTEMS.md(8a), stacked on #118: every piece of work off the Swing thread names its worker, and the project's write queue belongs to the change pipeline.Finish line
WriteQueueTest.closingFinishesTheWritesAlreadyTakenAndRefusesLaterOnesWriteQueueTest.closingWaitsThroughAnInterruptionAndKeepsItForTheCallerWriteQueueTest.anOfflineKeyChangeTakenBeforeClosingIsWrittenAndRecordedSystemsRulesTest(the shared-pool list is empty; the executors ofConfigChanges,JsonStateWriter,TextureThumbnails,ModLogoIconsandResourceViewPanelare gone from the thread list)What changes
WriteQueueis the project's write queue: a dedicated serial worker fromWorkers.projectWrites()that refuses writes once closed and waits, when closing, for those already taken. The project makes one for its pipeline and closes it before its change record, where it closedConfigChangesbefore.ChangePipeline.writeandwrites()give it to configuration settings andResourceEdits;ConfigChangeskeeps no thread. The queue's three tests moved fromConfigChangesTest; a regression also verifies writes and closing while all shared file workers are occupied.supplyAsyncand the like andForkJoinPool.commonPool(), run onWorkers.files().TextureThumbnailsandModLogoIconsread on aWorkers.fileStrand(), one at a time, as their archives stay on one thread;ResourceViewPanelreads on the file work;JsonStateWriter's delayed flush runs on the timer.ScriptFileActionsTest.movingWaitsForPendingSavesWithoutHoldingTheirWorkerverifies rename with three occupied workers and a pending save requiring the fourth.docs/SYSTEMS.mdnames the workers as built, and splits the last step into 8a (this), 8b (the current project as state) and 8c (owners noticing values put back, connection numbers).Left as it is
The debugger windows' reads now share the file work's four threads, where they shared the JVM's pool before; a debugger request that hangs holds one of them.
Focused
:companion:testchecks pass locally. CI runs the full build for the updated head.🤖 Generated with Claude Code