mirror of
https://github.com/discourse/discourse.git
synced 2026-08-06 13:08:40 +08:00
f0cbb92043I did a review pass over the TUI reporter and the step scheduler/coordinator and found a couple of bugs. The fixes are in separate commits, roughly by severity. The important one is in the step coordinator: the progress pipes and the chunk queue were created (and the parent-side writer ends closed) outside the fork mutex. The scheduler starts coordinators on their own threads, so a sibling step could fork in between and its children inherited those writer FDs. The pipes then never reached EOF until the foreign children exited — a step that finished in seconds stayed "running" for as long as a slow partitioned step next to it, and its forks were never returned to the budget. A leaked chunk-queue writer had the same effect on `claim`. Everything pipe-related now happens inside one fork-mutex block. The rest: - **TUI repaint**: the leftover-row accounting ignored the permanent lines that were printed in the same frame, so every step finish scrolled a spurious blank line at the bottom of the screen, and Ctrl-C left blank lines before the shell prompt (one per running step). - **Multi-line notices**: failure notices come from exception messages, which often contain newlines. An embedded `\n` broke the live-region cursor math and garbled the display. Notices are now split into one line per source line. - **`Tui#close`**: could raise or hang even though it runs inside the run's `ensure` — a crash in the render thread's own `ensure` (e.g. EPIPE from a dead output) re-raised through `join`, and a blocked write made the join wait forever. It now records the error, joins with a timeout, and restores the cursor best-effort. - **Scheduler teardown**: `await` reaps all workers before raising (no zombies, and the error names every crashed worker), `run` joins its threads and drains the consolidator even when something raises out of the wait loop, and a SignalException in a coordinator now records a failure so the summary doesn't render an empty error. Errors from that teardown land in the same bucket as shard-merge errors, so `merge_errors` is now called `finalization_errors` and the summary labels them as errors while finishing up instead of merge failures. - **Small cleanups**: coalesced progress entries could leak after a late update, `NO_COLOR=""` is no longer treated as set, a redundant color branch in the concurrency cell, `fps` validation, a shared helper for the summary/finished row columns, and `Ansi.truncate` no longer appends a stray RESET. - **`--max-parallel-steps`**: the help text now says what the flag actually does — it caps concurrent worker forks, and a partitioned step consumes several. No behavior change. - **ForkManager**: the fork-batching flag was process-global and only safe because the single caller serializes forking externally. It's thread-local now. **Schema sync** (last commit, unrelated to the rest — it just blocked `disco check` on this branch): core gained a `user_options.push_notification_level` column and a few new tables since the last sync. So I ignore the new tables (`nested_hot_post_scores`, `nested_hot_score_snapshots`, `sidebar_section_localizations`, `sidebar_url_localizations` — the tables they belong to are ignored already), regenerate the plugin manifest and the generated schema files, and pass the new column through in the Discourse converter. |
||
|---|---|---|
| .. | ||
| locales | ||
| intermediate_db.yml | ||
