Skip to content

Commit 12878d7

Browse files
cpovirkGoogle Java Core Libraries
authored andcommitted
Use toNanosSaturated in Queues.
While there, rename a parameter in a test helper method to match the production parameter name. I admit that my new test is not necessarily the best of all possible tests: It detects the problem currently, but it wouldn't detect the problem if we were to change the prod code to decompose the `Duration` only [when it actually needs to block](https://github.com/google/guava/blob/e416f493d7f6790cae859aab1ddacb456469f614/guava/src/com/google/common/collect/Queues.java#L336). I kind of doubt we'd ever bother, though. RELNOTES=n/a PiperOrigin-RevId: 828005669
1 parent 8cfa76f commit 12878d7

File tree

6 files changed

+125
-14
lines changed

6 files changed

+125
-14
lines changed

android/guava-tests/test/com/google/common/collect/QueuesTest.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import static org.junit.Assert.assertThrows;
2727

2828
import com.google.common.base.Stopwatch;
29+
import java.time.Duration;
2930
import java.util.ArrayList;
3031
import java.util.Collection;
3132
import java.util.List;
@@ -86,14 +87,14 @@ public void tearDown() throws InterruptedException {
8687
private static <T> int drain(
8788
BlockingQueue<T> q,
8889
Collection<? super T> buffer,
89-
int maxElements,
90+
int numElements,
9091
long timeout,
9192
TimeUnit unit,
9293
boolean interruptibly)
9394
throws InterruptedException {
9495
return interruptibly
95-
? Queues.drain(q, buffer, maxElements, timeout, unit)
96-
: Queues.drainUninterruptibly(q, buffer, maxElements, timeout, unit);
96+
? Queues.drain(q, buffer, numElements, timeout, unit)
97+
: Queues.drainUninterruptibly(q, buffer, numElements, timeout, unit);
9798
}
9899

99100
public void testMultipleProducers() throws Exception {
@@ -155,6 +156,15 @@ private void checkDrainTimesOut(BlockingQueue<Object> q) throws Exception {
155156
}
156157
}
157158

159+
public void testDrainOverflow() throws InterruptedException {
160+
Queues.drain(new SynchronousQueue<>(), new ArrayList<>(), /* numElements= */ 0, MAX_DURATION);
161+
}
162+
163+
public void testDrainUninterruptiblyOverflow() {
164+
Queues.drainUninterruptibly(
165+
new SynchronousQueue<>(), new ArrayList<>(), /* numElements= */ 0, MAX_DURATION);
166+
}
167+
158168
public void testZeroElements() throws Exception {
159169
for (BlockingQueue<Object> q : blockingQueues()) {
160170
checkZeroElements(q);
@@ -341,4 +351,6 @@ public void run() {
341351
}
342352
}
343353
}
354+
355+
private static final Duration MAX_DURATION = Duration.ofSeconds(Long.MAX_VALUE, 999_999_999);
344356
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* Copyright (C) 2019 The Guava Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
15+
package com.google.common.collect;
16+
17+
import com.google.common.annotations.GwtIncompatible;
18+
import com.google.common.annotations.J2ktIncompatible;
19+
import java.time.Duration;
20+
21+
/** This class is for {@code com.google.common.collect} use only! */
22+
@J2ktIncompatible
23+
@GwtIncompatible // java.time.Duration
24+
@IgnoreJRERequirement // We use this method only from within APIs that require a Duration.
25+
final class Internal {
26+
27+
/**
28+
* Returns the number of nanoseconds of the given duration without throwing or overflowing.
29+
*
30+
* <p>Instead of throwing {@link ArithmeticException}, this method silently saturates to either
31+
* {@link Long#MAX_VALUE} or {@link Long#MIN_VALUE}. This behavior can be useful when decomposing
32+
* a duration in order to call a legacy API which requires a {@code long, TimeUnit} pair.
33+
*/
34+
static long toNanosSaturated(Duration duration) {
35+
// Using a try/catch seems lazy, but the catch block will rarely get invoked (except for
36+
// durations longer than approximately +/- 292 years).
37+
try {
38+
return duration.toNanos();
39+
} catch (ArithmeticException tooBig) {
40+
return duration.isNegative() ? Long.MIN_VALUE : Long.MAX_VALUE;
41+
}
42+
}
43+
44+
private Internal() {}
45+
}

android/guava/src/com/google/common/collect/Queues.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.common.collect;
1616

17+
import static com.google.common.collect.Internal.toNanosSaturated;
1718
import static java.util.concurrent.TimeUnit.NANOSECONDS;
1819

1920
import com.google.common.annotations.GwtCompatible;
@@ -295,8 +296,7 @@ public static <E> SynchronousQueue<E> newSynchronousQueue() {
295296
public static <E> int drain(
296297
BlockingQueue<E> q, Collection<? super E> buffer, int numElements, Duration timeout)
297298
throws InterruptedException {
298-
// TODO(b/126049426): Consider using saturateToNanos(timeout) instead.
299-
return drain(q, buffer, numElements, timeout.toNanos(), NANOSECONDS);
299+
return drain(q, buffer, numElements, toNanosSaturated(timeout), NANOSECONDS);
300300
}
301301

302302
/**
@@ -365,8 +365,7 @@ public static <E> int drain(
365365
@IgnoreJRERequirement // Users will use this only if they're already using Duration
366366
public static <E> int drainUninterruptibly(
367367
BlockingQueue<E> q, Collection<? super E> buffer, int numElements, Duration timeout) {
368-
// TODO(b/126049426): Consider using saturateToNanos(timeout) instead.
369-
return drainUninterruptibly(q, buffer, numElements, timeout.toNanos(), NANOSECONDS);
368+
return drainUninterruptibly(q, buffer, numElements, toNanosSaturated(timeout), NANOSECONDS);
370369
}
371370

372371
/**

guava-tests/test/com/google/common/collect/QueuesTest.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import static org.junit.Assert.assertThrows;
2727

2828
import com.google.common.base.Stopwatch;
29+
import java.time.Duration;
2930
import java.util.ArrayList;
3031
import java.util.Collection;
3132
import java.util.List;
@@ -86,14 +87,14 @@ public void tearDown() throws InterruptedException {
8687
private static <T> int drain(
8788
BlockingQueue<T> q,
8889
Collection<? super T> buffer,
89-
int maxElements,
90+
int numElements,
9091
long timeout,
9192
TimeUnit unit,
9293
boolean interruptibly)
9394
throws InterruptedException {
9495
return interruptibly
95-
? Queues.drain(q, buffer, maxElements, timeout, unit)
96-
: Queues.drainUninterruptibly(q, buffer, maxElements, timeout, unit);
96+
? Queues.drain(q, buffer, numElements, timeout, unit)
97+
: Queues.drainUninterruptibly(q, buffer, numElements, timeout, unit);
9798
}
9899

99100
public void testMultipleProducers() throws Exception {
@@ -155,6 +156,15 @@ private void checkDrainTimesOut(BlockingQueue<Object> q) throws Exception {
155156
}
156157
}
157158

159+
public void testDrainOverflow() throws InterruptedException {
160+
Queues.drain(new SynchronousQueue<>(), new ArrayList<>(), /* numElements= */ 0, MAX_DURATION);
161+
}
162+
163+
public void testDrainUninterruptiblyOverflow() {
164+
Queues.drainUninterruptibly(
165+
new SynchronousQueue<>(), new ArrayList<>(), /* numElements= */ 0, MAX_DURATION);
166+
}
167+
158168
public void testZeroElements() throws Exception {
159169
for (BlockingQueue<Object> q : blockingQueues()) {
160170
checkZeroElements(q);
@@ -341,4 +351,6 @@ public void run() {
341351
}
342352
}
343353
}
354+
355+
private static final Duration MAX_DURATION = Duration.ofSeconds(Long.MAX_VALUE, 999_999_999);
344356
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* Copyright (C) 2019 The Guava Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
15+
package com.google.common.collect;
16+
17+
import com.google.common.annotations.GwtIncompatible;
18+
import com.google.common.annotations.J2ktIncompatible;
19+
import java.time.Duration;
20+
21+
/** This class is for {@code com.google.common.collect} use only! */
22+
@J2ktIncompatible
23+
@GwtIncompatible // java.time.Duration
24+
final class Internal {
25+
26+
/**
27+
* Returns the number of nanoseconds of the given duration without throwing or overflowing.
28+
*
29+
* <p>Instead of throwing {@link ArithmeticException}, this method silently saturates to either
30+
* {@link Long#MAX_VALUE} or {@link Long#MIN_VALUE}. This behavior can be useful when decomposing
31+
* a duration in order to call a legacy API which requires a {@code long, TimeUnit} pair.
32+
*/
33+
static long toNanosSaturated(Duration duration) {
34+
// Using a try/catch seems lazy, but the catch block will rarely get invoked (except for
35+
// durations longer than approximately +/- 292 years).
36+
try {
37+
return duration.toNanos();
38+
} catch (ArithmeticException tooBig) {
39+
return duration.isNegative() ? Long.MIN_VALUE : Long.MAX_VALUE;
40+
}
41+
}
42+
43+
private Internal() {}
44+
}

guava/src/com/google/common/collect/Queues.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.common.collect;
1616

17+
import static com.google.common.collect.Internal.toNanosSaturated;
1718
import static java.util.concurrent.TimeUnit.NANOSECONDS;
1819

1920
import com.google.common.annotations.GwtCompatible;
@@ -294,8 +295,7 @@ public static <E> SynchronousQueue<E> newSynchronousQueue() {
294295
public static <E> int drain(
295296
BlockingQueue<E> q, Collection<? super E> buffer, int numElements, Duration timeout)
296297
throws InterruptedException {
297-
// TODO(b/126049426): Consider using saturateToNanos(timeout) instead.
298-
return drain(q, buffer, numElements, timeout.toNanos(), NANOSECONDS);
298+
return drain(q, buffer, numElements, toNanosSaturated(timeout), NANOSECONDS);
299299
}
300300

301301
/**
@@ -363,8 +363,7 @@ public static <E> int drain(
363363
@GwtIncompatible // BlockingQueue
364364
public static <E> int drainUninterruptibly(
365365
BlockingQueue<E> q, Collection<? super E> buffer, int numElements, Duration timeout) {
366-
// TODO(b/126049426): Consider using saturateToNanos(timeout) instead.
367-
return drainUninterruptibly(q, buffer, numElements, timeout.toNanos(), NANOSECONDS);
366+
return drainUninterruptibly(q, buffer, numElements, toNanosSaturated(timeout), NANOSECONDS);
368367
}
369368

370369
/**

0 commit comments

Comments
 (0)