Explicitly separate state locks and awaits

[palfrey]

Description

#1981 made a number of changes around evicting_map and in particular there's some code where it does things around a lock and then drops the lock before doing .await calls. Unfortunately, the Clippy checks still flags https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock a few times around this, even though the sequence of actions is sound.

This PR explicitly separates the code doing anything with locks from the await code in a separate block, so it's clearer (both to humans and the Rust compiler) that it's split out. This lets us remove the manual drop's entirely. I can then enable the await_holding_lock lint in general.

There's also a few other minor linting fixes that clippy was warning about as well, mostly around iterators.

Type of change

Please delete options that aren't relevant.

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

bazel test //...

Checklist

• Updated documentation if needed
• Tests added/amended
• bazel test //... passes locally
• PR is contained in a single commit, using git amend see some docs

---

This change is 

[palfrey]

@chrisstaite Any thoughts on this one?

[chrisstaite-menlo]

Really nice, just a couple of optimisations.

Reviewable status: 0 of 1 LGTMs obtained, and 0 of 4 files reviewed, and 3 discussions need to be resolved

---

.bazelrc line 105 at r1 (raw file):

build --@rules_rust//:clippy_flag=-Dclippy::alloc_instead_of_core
build --@rules_rust//:clippy_flag=-Dclippy::as_underscore
build --@rules_rust//:clippy_flag=-Dclippy::await_holding_lock

Excellent.

---

nativelink-util/src/evicting_map.rs line 424 at r1 (raw file):

                }
            }
            let callbacks: FuturesUnordered<_> = removal_futures.into_iter().collect();

I think I'd prefer to simply move removal_futures from the scope as that's less work done under the lock.

---

nativelink-util/src/evicting_map.rs line 641 at r1 (raw file):

                };

                let removal_futures: FuturesUnordered<Pin<Box<dyn Future<Output = ()> + Send>>> =

As above, it would be better to do the work of collecting in to a FuturesUnordered outside of the lock.

[palfrey]

Reviewable status: 0 of 1 LGTMs obtained, and 0 of 4 files reviewed, and pending CI: macos-15, ubuntu-24.04, NativeLink.com Cloud / Remote Cache / macos-15, NativeLink.com Cloud / Remote Cache / ubuntu-24.04, integration-tests, ubuntu-24.04 / stable, windows-2022 / stable, pre-commit-checks, Analyze (javascript-typescript), Analyze (python), Coverage, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Bazel Dev / macos-15, Bazel Dev / ubuntu-24.04, Cargo Dev / macos-15, Cargo Dev / ubuntu-24.04, Installation / macos-15, Installation / ubuntu-24.04, buildstream, mongo, rbe-toolchain, asan / ubuntu-24.04, Local / bazel / ubuntu-24.04, and 3 discussions need to be resolved

---

nativelink-util/src/evicting_map.rs line 424 at r1 (raw file):

Previously, chrisstaite-menlo (Chris Staite) wrote…

I think I'd prefer to simply move removal_futures from the scope as that's less work done under the lock.

Done.

---

nativelink-util/src/evicting_map.rs line 641 at r1 (raw file):

Previously, chrisstaite-menlo (Chris Staite) wrote…

As above, it would be better to do the work of collecting in to a FuturesUnordered outside of the lock.

Done.

[chrisstaite-menlo]

Reviewable status: 0 of 1 LGTMs obtained, and 0 of 4 files reviewed, and pending CI: macos-15, ubuntu-24.04, NativeLink.com Cloud / Remote Cache / macos-15, NativeLink.com Cloud / Remote Cache / ubuntu-24.04, integration-tests, ubuntu-24.04 / stable, windows-2022 / stable, pre-commit-checks, Coverage, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Bazel Dev / macos-15, Bazel Dev / ubuntu-24.04, Cargo Dev / macos-15, Cargo Dev / ubuntu-24.04, Installation / macos-15, Installation / ubuntu-24.04, buildstream, mongo, rbe-toolchain, asan / ubuntu-24.04, Local / bazel / ubuntu-24.04, and 2 discussions need to be resolved

---

nativelink-util/src/evicting_map.rs line 649 at r2 (raw file):

        // Perform the async callbacks outside of the lock
        let mut removal_futures: FuturesUnordered<Pin<Box<dyn Future<Output = ()> + Send>>> =

You can use FuturesUnordered<_> here which is much nicer.

[palfrey]

Reviewable status: 0 of 1 LGTMs obtained, and 0 of 4 files reviewed, and pending CI: macos-15, ubuntu-24.04, NativeLink.com Cloud / Remote Cache / macos-15, NativeLink.com Cloud / Remote Cache / ubuntu-24.04, integration-tests, ubuntu-24.04 / stable, windows-2022 / stable, pre-commit-checks, Analyze (javascript-typescript), Analyze (python), Coverage, Bazel Dev / macos-15, Bazel Dev / ubuntu-24.04, Cargo Dev / macos-15, Cargo Dev / ubuntu-24.04, Installation / macos-15, Installation / ubuntu-24.04, buildstream, mongo, rbe-toolchain, asan / ubuntu-24.04, Local / bazel / ubuntu-24.04, and 2 discussions need to be resolved

---

nativelink-util/src/evicting_map.rs line 649 at r2 (raw file):

Previously, chrisstaite-menlo (Chris Staite) wrote…

You can use FuturesUnordered<_> here which is much nicer.

Done. I think this was a leftover from getting the vec![] case typed properly

[chrisstaite-menlo]

@chrisstaite-menlo reviewed 3 of 4 files at r1, 1 of 1 files at r3, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and all files reviewed, and pending CI: macos-15, ubuntu-24.04, NativeLink.com Cloud / Remote Cache / macos-15, NativeLink.com Cloud / Remote Cache / ubuntu-24.04, integration-tests, ubuntu-24.04 / stable, windows-2022 / stable, pre-commit-checks, Coverage, Bazel Dev / macos-15, Bazel Dev / ubuntu-24.04, Cargo Dev / macos-15, Cargo Dev / ubuntu-24.04, Installation / macos-15, Installation / ubuntu-24.04, buildstream, mongo, rbe-toolchain, asan / ubuntu-24.04, Local / bazel / ubuntu-24.04, and 1 discussions need to be resolved