Skip to content

fix(processlist): index io_counters instead of testing membership in it - #3675

Merged
nicolargo merged 2 commits into
nicolargo:developfrom
ntdatt812:fix/sum-stats-list-index
Aug 26, 2026
Merged

nicolargo merged 2 commits into
nicolargo:developfrom
ntdatt812:fix/sum-stats-list-index

Conversation

@ntdatt812

Copy link
Copy Markdown
Contributor

The defect

_sum_stats(key, sub_key) (glances/plugins/processlist/__init__.py:1030) guards the accumulation with a mapping test:

elif sub_key in p[key]:
    ret += p[key][sub_key]

sub_key is only a mapping key for memory_info. For io_counters it is an index — the value is the list [read_bytes, write_bytes, read_bytes_old, write_bytes_old, io_tag] (glances/processes.py:553) — so 0 in p['io_counters'] asks whether the counters contain the number zero.

Measured on develop:

stats = [[5000, 700, 2000, 300, 1],
         [3000, 400, 1000, 200, 1]]

_sum_stats('io_counters', 0)          ->    0     expected 8000
_sum_stats('io_counters', sub_key=2)  ->    0     expected 3000
_sum_stats('memory_info', 'rss')      ->   40     correct — it is a mapping

stats = [[5000, 700, 0, 300, 1]]
_sum_stats('io_counters', 0)          -> 5000     summed only because 0 is in the list

The symptom

The R/s and W/s figures on the process-list summary row — _msg_curse_sum, rendered whenever a process filter is active, for current/min/max — are computed as _sum_stats(…, 0) - _sum_stats(…, sub_key=2) at :945 and :955. Both terms come back 0 for every process actually doing IO, so the row reads zero; and when some idle process happens to carry the literal index in its counters, it reads a number that belongs to nobody. The VIRT/RES totals on the same row are right, which is what makes the zero look plausible.

The fix

Branch on the container type, and bounds-check the index so a short list is skipped rather than raising IndexError. The mapping path is untouched.

Verification

Windows 11, Python 3.14.7.

tests/test_core.py    47 passed, 6 skipped     (46 before — the case added)
ruff check            All checks passed

Mutation-checked, after confirming the edit applied: disabling the new branch fails the new case.

Tests

tests/test_core.py::test_010c_processlist_sum_stats_indexes_lists — all four io_counters indexes against known values, the mapping path unchanged (including an absent key), the "list contains the literal index" case that must no longer decide anything, and an out-of-range index that must be skipped rather than raise.

Note

The same shape exists at glances/folder_list.py:122,135 (i in self.timer_folders, an int against a list of Timers, so the per-folder folder_N_refresh option never takes effect). I left it out of this PR since it is a different plugin with a different symptom — happy to send it separately if useful.

`_sum_stats(key, sub_key)` guarded the accumulation with `sub_key in p[key]`.
That is a mapping test, and `sub_key` is only a mapping key for `memory_info`.
For `io_counters` it is an index — the value is the list

  [read_bytes, write_bytes, read_bytes_old, write_bytes_old, io_tag]

so `0 in p['io_counters']` asked whether the counters *contain the number zero*.
A process doing real IO was skipped, and one whose counters happened to hold the
literal 0/1/2/3 was summed instead:

  [[5000, 700, 2000, 300, 1],            _sum_stats('io_counters', 0) -> 0
   [3000, 400, 1000, 200, 1]]            (expected 8000)

  [[5000, 700, 0, 300, 1]]               _sum_stats('io_counters', 0) -> 5000
                                          (summed only because 0 is in the list)

So the R/s and W/s totals on the process-list summary row — rendered whenever a
process filter is active — were not merely zero but arbitrary. The VIRT and RES
totals beside them were correct, because `memory_info` really is a mapping.

Branch on the container type, and bounds-check the index so a short list is
skipped rather than raising.
@nicolargo
nicolargo merged commit 2da4ccb into nicolargo:develop Aug 26, 2026
3 of 5 checks passed
@nicolargo

Copy link
Copy Markdown
Owner

Thanks for the PR @ntdatt812

Merged into develop (i solve a conflict in the test file).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants