From 3e373d0abc79e40b3fa46aca4aa268c5ae9b7dc2 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 3 Jan 2024 09:16:20 -0800 Subject: [PATCH] Use long instead of int for thread indices This protects against an integer overflow which could occur for large key list size and large thread counts. Regrettably, it's difficult to write a regression test for this scenario, as exercising this overflow requires lots of time and heap, so it would be a performance regression to our test suites. Fixes https://github.com/bazelbuild/bazel/issues/19445 PiperOrigin-RevId: 595420516 Change-Id: Ic0a475a6a273c50fe9895dd0852fa5b062859cb2 --- .../build/skyframe/InvalidatingNodeVisitor.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java index e79d88348d0eea..51aa4e74741aee 100644 --- a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java +++ b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java @@ -274,18 +274,18 @@ protected void runInternal(ImmutableList> pending MIN_TIME_FOR_LOGGING)) { // To avoid contention and scheduling too many jobs for our #cpus, we start // DEFAULT_THREAD_COUNT jobs, each processing a chunk of the pending visitations. - int listSize = pendingList.size(); - int numThreads = min(DEFAULT_THREAD_COUNT, listSize); - for (int i = 0; i < numThreads; i++) { - int index = i; + long listSize = pendingList.size(); + long numThreads = min(DEFAULT_THREAD_COUNT, listSize); + for (long i = 0; i < numThreads; i++) { + // Use long multiplication to avoid possible overflow, as numThreads * listSize might be + // larger than max int. + int startIndex = (int) ((i * listSize) / numThreads); + int endIndex = (int) (((i + 1) * listSize) / numThreads); executor.execute( () -> visit( Collections2.transform( - pendingList.subList( - (index * listSize) / numThreads, - ((index + 1) * listSize) / numThreads), - Pair::getFirst), + pendingList.subList(startIndex, endIndex), Pair::getFirst), InvalidationType.DELETED)); } }