tracemalloc: _compare_grouped_stats mutates old_group, and a typo in _group_by

[Tony123-tech]

Two small issues in Lib/tracemalloc.py:

1. _compare_grouped_stats() mutates its old_group argument by popping
   keys from it. After the call, old_group is missing every key that
   was also in new_group.

2. A typo in the error message raised by _group_by():
   "cumulative mode cannot by used" should be
   "cumulative mode cannot be used".

Reproduction for (1):

from tracemalloc import Statistic, _compare_grouped_stats
old = {"a": Statistic("a", 100, 2), "b": Statistic("b", 50, 1)}
new = {"a": Statistic("a", 200, 3), "c": Statistic("c", 10, 1)}
old_copy = dict(old)
_compare_grouped_stats(old, new)
print(len(old), len(old_copy))   # 1 2

Linked PRs

• gh-158234: Fix typo in tracemalloc cumulative mode error message #158233

[StanFromIreland]

Can 1 be reproduced using only public APIs?

[Tony123-tech]

No, it cannot be reproduced using only public APIs. The caller,
Snapshot.compare_to(), does not reuse the old_group dict, so the
mutation is currently harmless.

The issue is that _compare_grouped_stats() modifies its argument,
which is a bad pattern even if no caller is currently affected. The
fix is a one-line defensive copy:

old_group = dict(old_group)

If you think this isn't worth fixing because it's a private function
with no affected public API, feel free to close it.

[picnixz]

We reject defensive programming in general if this cannot be reached publicly. Callers are consenting adults.

[picnixz]

However the fix for the exception is real I think so let's keep it.

[picnixz]

After looking at the PR I think we can still make the change just in case as the name of the function may suggest a pure function.

[Tony123-tech]

Thanks for the review! I understand the concern about defensive programming. I'll keep the change since the function name suggests it's pure, and the typo fix is clearly correct.

[Tony123-tech]

Thanks for the review! I understand the concern about defensive programming. I'll keep the change since the function name suggests it's pure, and the typo fix is clearly correct.