processes: sort the process list on converted values (fix #3830)

glances/plugins/processlist/model.py

@@ -189,9 +189,10 @@ def __init__(self, store: StatsStore, config: GlancesConfig) -> None:
     # publishing it whole would duplicate a dozen fields already in `data[]`
     # for the same pid.
     #
-    # `cmdline` titles the block, with its arguments, on both surfaces. The
-    # engine merges it into the process after the extended grab, so it copies
-    # it into `extended_process` separately (`GlancesProcesses.update`).
+    # `cmdline` titles the block, with its arguments, on both surfaces. It
+    # reaches `extended_process` with the rest of the process dict — the
+    # engine merges the cached attrs before the extended grab, in the
+    # pre-sort metadata pass (`GlancesProcesses.update`).
     _EXTENDED_KEYS: ClassVar[tuple[str, ...]] = (
         "pid",
         "name",

glances/processes.py

@@ -684,16 +684,39 @@ def update(self):
         # Build the process list
         processlist = self.build_process_list(sorted_attrs)
 
+        # Update the processcount (on the raw status strings: the metadata
+        # pass below truncates 'status' to its first letter for display)
+        self.update_processcount(processlist)
+
+        # Loop over the processes to materialize the fields a sort key can
+        # point at, BEFORE sorting. `io_counters` is still a psutil namedtuple
+        # here — indexes 2/3 are read/write *counts*, not the previous-cycle
+        # byte totals the sort lambda subtracts — and `cpu_times` a pcputimes
+        # namedtuple with no .get(), which made the sort raise and silently
+        # fall back to cpu_percent order (issue #3830). The cached attrs
+        # (username, cmdline) only exist on each proc after this pass as well.
+        for proc in processlist:
+            # Meta data
+            ###########
+            proc = self.get_pid_time_and_status(time_since_update, proc)
+
+            # Process IO
+            proc = self.get_io_counters(proc)
+
+            # Manage cached information
+            proc = self.maybe_add_cached_stats(is_cached, cached_attrs, proc)
+
+            # cpu_times reaches the sort (and consumers) as a dict, the shape
+            # update_list produces with namedtuple_to_dict
+            cpu_times = proc.get('cpu_times')
+            if hasattr(cpu_times, '_asdict'):
+                proc['cpu_times'] = cpu_times._asdict()
+
         # Sort the processes list by the current sort_key
         # This is needed so that cursor_position matches the displayed order (see issue #3400)
         processlist = sort_stats(processlist, sorted_by=self.sort_key, reverse=self.sort_reverse)
 
-        # Update the processcount
-        self.update_processcount(processlist)
-
-        # Loop over processes and :
-        # - add extended stats for selected process
-        # - add metadata
+        # Loop over processes and add extended stats for the selected process
         # Whether the pinned process was still running this cycle. Without it,
         # a process that exits leaves `extended_process` holding its last
         # values forever -- nothing below ever runs again for a pid that is no
@@ -719,21 +742,6 @@ def update(self):
                 self.extended_process = namedtuple_to_dict(proc)
                 extended_seen = True
 
-            # Meta data
-            ###########
-            proc = self.get_pid_time_and_status(time_since_update, proc)
-
-            # Process IO
-            proc = self.get_io_counters(proc)
-
-            # Manage cached information
-            proc = self.maybe_add_cached_stats(is_cached, cached_attrs, proc)
-
-            # The command line is only merged above, after the extended grab:
-            # give it to the pinned process so its title can show it.
-            if self.extended_process is not None and proc['pid'] == self.extended_process['pid']:
-                self.extended_process['cmdline'] = proc.get('cmdline')
-
         # The pinned process is gone. Forget it, rather than leave the TUI and
         # the WebUI showing its last numbers under a `extended_stats: True`
         # that is no longer true. The PIN goes with the accumulator: a pin on

tests/test_processes_extended.py

@@ -236,8 +236,9 @@ def test_a_new_pin_starts_from_fresh_min_max_mean(monkeypatch):
 
 
 def test_the_pinned_process_carries_its_command_line(monkeypatch):
-    """`cmdline` is merged into the process after the extended grab; the
-    pinned process must still carry it, as it titles the block."""
+    """`cmdline` is merged into the process before the extended grab, in the
+    pre-sort metadata pass; the pinned process must still carry it, as it
+    titles the block."""
     import os
 
     pid = os.getpid()

tests/test_processes_sort_order.py

@@ -0,0 +1,127 @@
+"""`update()` must sort the process list on post-conversion values.
+
+`build_process_list` returns raw `as_dict()` rows: `cpu_times` is a
+``pcputimes`` namedtuple and `io_counters` a ``pio`` one — psutil's
+``pio(read_count, write_count, read_bytes, write_bytes, ...)`` puts the
+*counts* at indexes 0/1, which is why `get_io_counters` reads bytes at
+indexes 2/3. The engine used to sort that raw list before the per-process
+loop ran `get_io_counters` / `namedtuple_to_dict`, so
+`--sort-processes io_counters` subtracted the byte totals as if they were
+the previous cycle's values, and `cpu_times` sorting raised inside
+`sorted()` and silently fell back to `cpu_percent` order (issue #3830).
+The REST API and WebUI serve this engine order directly (`get_list()`);
+only the TUI re-sorted later.
+"""
+
+from collections import namedtuple
+
+import pytest
+
+from glances.processes import GlancesProcesses
+
+# Raw psutil shapes, as `p.as_dict()` produces them.
+pcputimes = namedtuple('pcputimes', ['user', 'system', 'children_user', 'children_system'])
+pio = namedtuple('pio', ['read_count', 'write_count', 'read_bytes', 'write_bytes'])
+
+
+def _raw_proc(pid, name, cpu_percent, cpu_times, io_counters, memory_percent=1.0):
+    """A process row in the shape `build_process_list` emits it (pre-conversion)."""
+    return {
+        'pid': pid,
+        'name': name,
+        'cmdline': [name],
+        'cpu_percent': cpu_percent,
+        'memory_percent': memory_percent,
+        'memory_info': {'rss': 0, 'vms': 0},
+        'status': 'sleeping',
+        'num_threads': 1,
+        'nice': 0,
+        'username': 'root',
+        'cpu_times': cpu_times,
+        'io_counters': io_counters,
+        'gids': {'real': 0},
+    }
+
+
+@pytest.fixture
+def engine():
+    return GlancesProcesses()
+
+
+def test_sort_by_cpu_times_orders_on_user_plus_system(engine, monkeypatch):
+    # The busiest-by-time process has the LOWEST cpu_percent: a fallback to
+    # cpu_percent order (what the pre-fix sort silently did) inverts the list.
+    rows = [
+        _raw_proc(1, 'highest-cpu', 99.0, pcputimes(1.0, 0.0, 0.0, 0.0), None),
+        _raw_proc(2, 'long-runner', 1.0, pcputimes(500.0, 40.0, 0.0, 0.0), None),
+        _raw_proc(3, 'mid', 5.0, pcputimes(100.0, 10.0, 0.0, 0.0), None),
+    ]
+    monkeypatch.setattr(engine, 'build_process_list', lambda attrs: rows)
+    engine.set_sort_key('cpu_times', auto=False)
+
+    engine.update()
+
+    assert [p['name'] for p in engine.processlist] == ['long-runner', 'mid', 'highest-cpu']
+
+
+def test_sort_by_io_counters_orders_on_bytes_since_last_cycle(engine, monkeypatch):
+    # The writer performs many small writes: its write *count* dwarfs nothing,
+    # but its byte totals are huge — sorting the raw namedtuple subtracts the
+    # bytes as if they were last cycle's, sending it to the bottom (the
+    # reported symptom).
+    rows = [
+        _raw_proc(1, 'idle-reader', 0.0, pcputimes(0.0, 0.0, 0.0, 0.0), pio(50, 40, 50, 500)),
+        _raw_proc(2, 'heavy-writer', 0.0, pcputimes(0.0, 0.0, 0.0, 0.0), pio(1, 90000, 100, 10_000_000)),
+        _raw_proc(3, 'no-io', 0.0, pcputimes(0.0, 0.0, 0.0, 0.0), None),
+    ]
+    monkeypatch.setattr(engine, 'build_process_list', lambda attrs: rows)
+    engine.set_sort_key('io_counters', auto=False)
+
+    engine.update()
+
+    assert [p['name'] for p in engine.processlist] == ['heavy-writer', 'idle-reader', 'no-io']
+
+
+def test_sort_by_io_counters_uses_the_rate_not_lifetime_bytes(engine, monkeypatch):
+    # Second cycle: 'writer' added few new bytes while 'bursty' added many.
+    # Only post-conversion rows (with the previous cycle's bytes merged in)
+    # can order by rate.
+    cycle = iter(
+        [
+            [
+                _raw_proc(1, 'writer', 0.0, pcputimes(0.0, 0.0, 0.0, 0.0), pio(1, 10, 0, 10_000_000)),
+                _raw_proc(2, 'bursty', 0.0, pcputimes(0.0, 0.0, 0.0, 0.0), pio(1, 10, 0, 1_000)),
+            ],
+            [
+                _raw_proc(1, 'writer', 0.0, pcputimes(0.0, 0.0, 0.0, 0.0), pio(1, 10, 0, 10_100_000)),
+                _raw_proc(2, 'bursty', 0.0, pcputimes(0.0, 0.0, 0.0, 0.0), pio(1, 10, 0, 5_000_000)),
+            ],
+        ]
+    )
+    monkeypatch.setattr(engine, 'build_process_list', lambda attrs: next(cycle))
+    engine.set_sort_key('io_counters', auto=False)
+
+    engine.update()
+    assert [p['name'] for p in engine.processlist] == ['writer', 'bursty']
+
+    engine.update()
+    assert [p['name'] for p in engine.processlist] == ['bursty', 'writer']
+
+
+def test_engine_order_matches_a_post_conversion_resort(engine, monkeypatch):
+    """The API serves `get_list()` unsorted: the engine order must already be
+    what `get_list(sorted=True)` — the post-conversion re-sort — produces."""
+    rows = [
+        _raw_proc(1, 'a', 50.0, pcputimes(1.0, 1.0, 0.0, 0.0), pio(5, 5, 10, 10)),
+        # Biggest lifetime writer: raw-order (count - bytes) demotes 'b' last.
+        _raw_proc(2, 'b', 1.0, pcputimes(300.0, 0.0, 0.0, 0.0), pio(5, 9000, 10, 10_000_000)),
+        _raw_proc(3, 'c', 10.0, pcputimes(20.0, 5.0, 0.0, 0.0), pio(5, 50, 10, 40)),
+    ]
+    monkeypatch.setattr(engine, 'build_process_list', lambda attrs: rows)
+    engine.set_sort_key('io_counters', auto=False)
+
+    engine.update()
+    api_order = [p['name'] for p in engine.get_list()]
+    resorted = [p['name'] for p in engine.get_list(sorted=True)]
+
+    assert api_order == resorted == ['b', 'c', 'a']