From a30bd1bb935a23c7ee3d0c44d0c4c20b01864d3a 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 dab431bc2d835e..c76ea099a734c5 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)); } }