Programming Languages Bug Report

Rust E0502 updating a HashMap while iterating it, and the clone that hurt

E0502 stopped us from updating parent totals inside a HashMap loop, so someone cloned the whole map to make it compile. That clone ran every ten seconds over 1.2 million entries. Here is why the borrow checker was right and the fixes that kept memory flat.

Daniel Tran Daniel Tran @fun_dev · April 15, 2025 · 10 min read · 5 Views
Banner: a Rust hash map with a read lock and a write lock pointing at it at the same time

Our usage-metering service collects API request counts per account and, every ten seconds, rolls child accounts up into their parent organisation so that rate limits and billing can work at the org level. After a release in late March, the service's memory started following a sawtooth: resident memory climbed by around 180 MB every tick, dropped, and climbed again. p99 latency for the /v1/usage endpoint went from 4 ms to 160 ms during each tick, and one pod was OOM-killed on the first busy afternoon.

The release notes for that version said only "refactor rollup to update totals in place". The diff contained this line:

for (account, u) in usage.clone().iter() {

Reproducing it

The developer who made the change explained what happened. The original goal was to add each child account's usage to its parent's entry in the same HashMap. The first attempt did not compile:

error[E0502]: cannot borrow `*usage` as mutable because it is also borrowed as immutable
 --> src/rollup.rs:9:21
  |
7 |     for (account, u) in usage.iter() {
  |                         ------------
  |                         |
  |                         immutable borrow occurs here
  |                         immutable borrow later used here
8 |         if let Some(parent) = parents.get(account) {
9 |             let p = usage.get_mut(parent).unwrap();
  |                     ^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here

Adding .clone() made the error disappear, tests passed on a fixture of 20 accounts, and it shipped. In production the map holds about 1.2 million accounts with String keys, so every tick allocated and freed a full copy of it.

What we ruled out

  • An allocator problem. We tried jemalloc in a canary pod. Peak memory was slightly lower, but the sawtooth and the latency spikes were identical.
  • Lock contention. The map sits behind a tokio::sync::RwLock. Holding the write lock during the rollup does block readers, but the old code held it for under 3 ms. The new code held it for around 140 ms, almost all of it spent cloning.
  • Traffic growth. Request volume was flat week over week.

Root cause

The clone was a workaround for a borrow-checker error that was pointing at a real design problem. usage.iter() borrows the map immutably for the whole loop, and usage.get_mut() needs a mutable borrow of the same map, which Rust forbids. Cloning gave the loop its own copy to iterate, so the error went away, but at the cost of an O(n) allocation every ten seconds. Worse, the in-place update had a logic bug the clone hid: because parent and child entries lived in the same map, a parent's own total now included its children, and anything reading "per-account usage" for a parent account got double counts.

What is happening under the hood

Rust's borrowing rules allow either any number of shared references (&T) or exactly one mutable reference (&mut T) to a value at a time. An iterator from HashMap::iter() holds a shared reference to the map until the loop ends, and each u it yields is a reference into the map's storage.

If Rust allowed get_mut or insert during that loop, an insert could trigger a resize, which moves every entry to a new allocation. The iterator and u would then point at freed memory. In languages without a borrow checker this is the iterator invalidation bug; in Java it shows up as a ConcurrentModificationException at runtime. Rust rejects it at compile time with E0502.

Even mutating an existing value without resizing is rejected, because the compiler cannot prove that parent is not the same key as account, which would create a &mut and a & to the same Usage at once.

Environment

Component Version / value
OS Debian 12 container on Kubernetes 1.31
Rust 1.86.0, edition 2021
Async runtime tokio 1.44
Data HashMap<String, Usage>, about 1.2 million entries
Tick interval 10 seconds
Pod memory limit 1 GiB

Minimal reproduction

use std::collections::HashMap;

#[derive(Debug, Default, Clone)]
struct Usage { requests: u64, bytes: u64 }

fn rollup(usage: &mut HashMap<String, Usage>, parents: &HashMap<String, String>) {
    for (account, u) in usage.iter() {
        if let Some(parent) = parents.get(account) {
            let p = usage.get_mut(parent).unwrap(); // E0502
            p.requests += u.requests;
            p.bytes += u.bytes;
        }
    }
}

Who is affected

Every Rust version. The pattern appears whenever code reads from a collection and writes to the same collection in one pass: graph updates, parent-child rollups, moving values between two keys, or updating a Vec element based on another element of the same Vec.

The fix

We went back to what the code was actually meant to compute: per-account usage is one thing, per-org totals are another. They now live in separate fields of a struct. Rust tracks borrows of struct fields separately when you access them directly, so iterating self.usage while mutating self.org_totals compiles without any copying:

#[derive(Debug, Default, Clone, PartialEq)]
pub struct Usage { pub requests: u64, pub bytes: u64 }

#[derive(Default)]
pub struct Ledger {
    usage: HashMap<String, Usage>,
    org_totals: HashMap<String, Usage>,
}

impl Ledger {
    pub fn rollup(&mut self, parents: &HashMap<String, String>) {
        self.org_totals.clear(); // keeps capacity, no reallocation per tick
        for (account, u) in &self.usage {
            let org = parents.get(account).unwrap_or(account);
            let t = self.org_totals.entry(org.clone()).or_default();
            t.requests += u.requests;
            t.bytes += u.bytes;
        }
    }
}

The entry API handles "insert if missing, otherwise update" with a single lookup, which also replaced the unwrap() that would have panicked for an org with no entry yet.

-for (account, u) in usage.clone().iter() {
-    if let Some(parent) = parents.get(account) {
-        let p = usage.get_mut(parent).unwrap();
+for (account, u) in &self.usage {
+    let org = parents.get(account).unwrap_or(account);
+    let t = self.org_totals.entry(org.clone()).or_default();

Note that the field split only works on direct field access. If you call a &mut self method inside the loop, the compiler sees a borrow of all of self and the error comes back.

Alternatives and trade-offs

  1. Two phases: collect, then apply. Gather the changes into a Vec while borrowing immutably, end the loop, then apply them. The extra memory is proportional to the number of changes, not the map size. This is our default when the updates really must go into the same map.

    let deltas: Vec<(String, Usage)> = usage
        .iter()
        .filter_map(|(a, u)| parents.get(a).map(|p| (p.clone(), u.clone())))
        .collect();
    for (parent, d) in deltas {
        let p = usage.entry(parent).or_default();
        p.requests += d.requests;
        p.bytes += d.bytes;
    }
    
  2. iter_mut or values_mut when each update only touches the current entry, for example decaying every counter by 10%. No second lookup, no conflict.

  3. HashMap::get_disjoint_mut, stabilised in Rust 1.86.0, returns mutable references to several distinct keys at once. We use it for moving quota between two accounts. It panics if the keys overlap, so check that first:

    fn move_quota(q: &mut HashMap<String, u64>, from: &str, to: &str, n: u64) -> bool {
        if from == to { return false; }
        let [Some(src), Some(dst)] = q.get_disjoint_mut([from, to]) else { return false };
        if *src < n { return false; }
        *src -= n;
        *dst += n;
        true
    }
    
  4. Indices into a Vec. Store values in a Vec<Usage> and keep a HashMap<String, usize> from key to index. Index-based access sidesteps long-lived borrows and is cache friendly, at the cost of managing removals yourself.

  5. RefCell or Cell inside the values. Moves the check to runtime. It compiles, but a mistake becomes a panic in production instead of a compile error, so we avoid it for this case.

How to test the fix

A unit test asserts the semantics that the clone version got wrong: per-account usage stays untouched and org totals include every child exactly once.

#[test]
fn rollup_does_not_double_count_parents() {
    let mut l = Ledger::default();
    l.usage.insert("org_1".into(), Usage { requests: 1, bytes: 1 });
    l.usage.insert("acct_a".into(), Usage { requests: 3, bytes: 10 });
    l.usage.insert("acct_b".into(), Usage { requests: 2, bytes: 5 });
    let parents = HashMap::from([
        ("acct_a".to_string(), "org_1".to_string()),
        ("acct_b".to_string(), "org_1".to_string()),
    ]);

    l.rollup(&parents);
    l.rollup(&parents); // running twice must not accumulate

    assert_eq!(l.org_totals["org_1"], Usage { requests: 6, bytes: 16 });
    assert_eq!(l.usage["org_1"], Usage { requests: 1, bytes: 1 });
}

We also added a Criterion benchmark with 1.2 million synthetic accounts:

cargo test rollup
cargo bench --bench rollup

The clone version took about 135 ms per rollup in the benchmark; the field-split version took 31 ms with no per-tick allocations once org_totals had reached its working capacity.

Confirming in production

After the deploy the memory sawtooth disappeared. Resident memory settled at 410 MB and stayed flat across ticks. Write-lock hold time during the rollup, which we now export as a histogram, dropped from about 140 ms to 28 ms at p99, and /v1/usage p99 latency returned to 4 to 5 ms. There have been no OOM kills since. Finance re-ran the March org reports, which corrected a 3.4% overcount on parent accounts that had their own direct usage.

Regression guard

  • Clippy's redundant_clone lint does not catch this case, because the clone is technically needed for the code as written. Instead we added a review rule: any .clone() on a collection inside a hot path needs a comment explaining why.
  • The Criterion benchmark runs in CI on a fixed runner, and a regression of more than 25% fails the job.
  • A Prometheus alert fires if the rollup's lock hold time p99 exceeds 50 ms.

Security implications

None beyond availability. The clone did not expose data, but the OOM kill dropped in-memory counters for one pod, which for a metering service means unbilled usage and briefly unenforced rate limits. Rate limiting that fails open under memory pressure is a denial-of-service lever, so we also persisted counters every tick and made the limiter fail closed for unknown accounts.

Prevention checklist

  • Treat E0502 as a design question first: are you mixing two different pieces of state in one collection?
  • Prefer the two-phase pattern, the entry API or field splitting over cloning a collection.
  • Use get_disjoint_mut for multi-key updates on Rust 1.86 or later, and guard against equal keys.
  • Test with production-sized data, not only small fixtures, when a change touches a hot loop.
  • Export lock hold times for any lock held during periodic batch work.

References

Add a label

#rust #borrow-checker #hashmap #e0502 #performance
Written by
Daniel Tran @fun_dev

I have spent most of my career on the side of software that pages people at night: payment integrations, web platforms with real traffic, CI/CD pipelines, Kubernetes clusters and the databases underneath them. Somewhere along the way I started keeping a notebook of every bug that cost more than an hour — the exact error, what we ruled out, the real root cause and how we proved the fix. Hibug is where those notes live. Every post follows the same path: Investigate, Explain, Fix, Verify — with the environment spelled out, the fix tested, and the security angle checked.

Advertisement

Comments

0

No comments yet. Be the first to comment!

Sign In to post a comment.