From b351c8e4672ffc1e65a023a86bd3864018eb315f Mon Sep 17 00:00:00 2001 From: roboomp Date: Mon, 27 Jul 2026 03:36:22 +0000 Subject: [PATCH] fix(natives): surfaced sort worker panic instead of silent truncation Returning end-of-input from chunks::read on a disconnected receiver also masked a panicking ext_sort sorter thread (comparator or Rayon panic in sort_by), letting sort exit 0 with truncated or empty output. Retain the sorter JoinHandle and join it after read_write_loop, mapping a thread panic to an error; drop the sorted-chunk receiver first so a still-running sorter unblocks instead of deadlocking the join. Added an external multi-chunk sort test covering the spill-to-files join path. Fixes #6736 --- .../vendor/uu-sort/src/ext_sort/threaded.rs | 59 ++++++++++++++++++- packages/natives/CHANGELOG.md | 2 +- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/crates/vendor/uu-sort/src/ext_sort/threaded.rs b/crates/vendor/uu-sort/src/ext_sort/threaded.rs index 54d240b56..879d88839 100644 --- a/crates/vendor/uu-sort/src/ext_sort/threaded.rs +++ b/crates/vendor/uu-sort/src/ext_sort/threaded.rs @@ -16,7 +16,7 @@ use std::{ use flume::{Receiver, Sender}; use itertools::Itertools; -use uucore::error::{UResult, strip_errno}; +use uucore::error::{UResult, USimpleError, strip_errno}; use crate::{ GlobalSettings, Line, Output, @@ -46,7 +46,7 @@ pub fn ext_sort( ) -> UResult<()> { let (sorted_sender, sorted_receiver) = flume::bounded(1); let (recycled_sender, recycled_receiver) = flume::bounded(1); - thread::spawn({ + let sorter_handle = thread::spawn({ let settings = settings.clone(); move || sorter(&recycled_receiver, &sorted_sender, &settings) }); @@ -77,7 +77,7 @@ pub fn ext_sort( } } - if effective_settings.compress_prog.is_some() { + let result = if effective_settings.compress_prog.is_some() { reader_writer::<_, WriteableCompressedTmpFile>( files, &effective_settings, @@ -95,6 +95,23 @@ pub fn ext_sort( output, tmp_dir, ) + }; + + // Drop our end of the sorted-chunk channel so a still-running sorter (e.g. + // after `reader_writer` bailed on an I/O error) unblocks its pending send and + // exits, instead of deadlocking the join below. + drop(sorted_receiver); + + // Surface a sorter-thread panic (e.g. a comparator or Rayon panic inside + // `sort_by`) as an error. `chunks::read` now reports the sorter's + // disconnection as end-of-input (issue #6736), so without joining here a + // discarded panic would masquerade as a successful short read and let `sort` + // exit 0 with truncated or empty output. + match sorter_handle.join() { + Ok(()) => result, + Err(_) => { + result.and(Err(USimpleError::new(2, "sort: sorter thread terminated unexpectedly".to_string()))) + }, } } @@ -295,3 +312,39 @@ fn write_lines(lines: &[Line], writer: &mut T, separator: u8) { writer.write_all(&[separator]).unwrap(); } } + +#[cfg(test)] +mod tests { + use std::io::{Cursor, Read}; + + use super::*; + + /// External (multi-chunk) sort must run to completion and emit fully sorted + /// output. Regression guard for #6760: `ext_sort` now joins the sorter thread + /// after `read_write_loop`. A tiny explicit buffer forces spilling to + /// temporary files, so the join runs on the `WroteChunksToFile` path — it + /// must surface sorted output rather than deadlock or truncate. + #[test] + fn ext_sort_spills_to_files_and_sorts() { + let input: String = (0..200u32).rev().map(|i| format!("{i:04}\n")).collect(); + + let mut settings = GlobalSettings::default(); + settings.buffer_size = 64; + settings.buffer_size_is_explicit = true; + + let out_dir = tempfile::tempdir().expect("temp dir"); + let out_path = out_dir.path().join("sorted.txt"); + + let mut files = std::iter::once(Ok( + Box::new(Cursor::new(input.into_bytes())) as Box, + )); + let output = Output::new(Some(out_path.as_os_str())).expect("open output"); + let mut tmp_dir = TmpDirWrapper::new(std::env::temp_dir()); + + ext_sort(&mut files, &settings, output, &mut tmp_dir).expect("ext_sort succeeds"); + + let sorted = std::fs::read_to_string(&out_path).expect("read output"); + let expected: String = (0..200u32).map(|i| format!("{i:04}\n")).collect(); + assert_eq!(sorted, expected); + } +} diff --git a/packages/natives/CHANGELOG.md b/packages/natives/CHANGELOG.md index 2111f80aa..055d45f82 100644 --- a/packages/natives/CHANGELOG.md +++ b/packages/natives/CHANGELOG.md @@ -6,7 +6,7 @@ ### Fixed -- Fixed the native `sort` builtin panicking with `SendError(..)` at `chunks.rs:248` when the chunk-channel receiver disconnected early (e.g. a consumer thread stopping after an error or closed output); the reader now stops gracefully instead of unwrapping the failed send ([#6736](https://github.com/can1357/oh-my-pi/issues/6736)). +- Fixed the native `sort` builtin panicking with `SendError(..)` at `chunks.rs:248` when the chunk-channel receiver disconnected early (e.g. a consumer thread stopping after an error or closed output); the reader now stops gracefully instead of unwrapping the failed send, and a panicking external-sort worker thread is surfaced as an error instead of silently emitting truncated output ([#6736](https://github.com/can1357/oh-my-pi/issues/6736)). ## [17.1.4] - 2026-07-26