Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8273369: Computing micros between two instants unexpectedly overflows for some cases #5396

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/java.base/share/classes/java/time/Instant.java
Expand Up @@ -61,6 +61,7 @@
*/
package java.time;

import static java.time.LocalTime.MICROS_PER_SECOND;
import static java.time.LocalTime.NANOS_PER_SECOND;
import static java.time.LocalTime.SECONDS_PER_DAY;
import static java.time.LocalTime.SECONDS_PER_HOUR;
Expand Down Expand Up @@ -1145,7 +1146,7 @@ public long until(Temporal endExclusive, TemporalUnit unit) {
if (unit instanceof ChronoUnit chronoUnit) {
return switch (chronoUnit) {
case NANOS -> nanosUntil(end);
case MICROS -> nanosUntil(end) / 1000;
case MICROS -> microsUntil(end);
case MILLIS -> Math.subtractExact(end.toEpochMilli(), toEpochMilli());
case SECONDS -> secondsUntil(end);
case MINUTES -> secondsUntil(end) / SECONDS_PER_MINUTE;
Expand All @@ -1164,6 +1165,12 @@ private long nanosUntil(Instant end) {
return Math.addExact(totalNanos, end.nanos - nanos);
}

private long microsUntil(Instant end) {
long secsDiff = Math.subtractExact(end.seconds, seconds);
long totalMicros = Math.multiplyExact(secsDiff, MICROS_PER_SECOND);
return Math.addExact(totalMicros, (end.nanos - nanos) / 1000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you define NANOS_PER_MICRO, the others conversions use predefined constants.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defined it as a private field in Instant.

}

private long secondsUntil(Instant end) {
long secsDiff = Math.subtractExact(end.seconds, seconds);
long nanosDiff = end.nanos - nanos;
Expand Down
6 changes: 5 additions & 1 deletion src/java.base/share/classes/java/time/LocalTime.java
Expand Up @@ -186,10 +186,14 @@ public final class LocalTime
* Milliseconds per day.
*/
static final long MILLIS_PER_DAY = SECONDS_PER_DAY * 1000L;
/**
* Microseconds per second.
*/
static final long MICROS_PER_SECOND = 1000_000L;
/**
* Microseconds per day.
*/
static final long MICROS_PER_DAY = SECONDS_PER_DAY * 1000_000L;
static final long MICROS_PER_DAY = MICROS_PER_SECOND * SECONDS_PER_DAY;
/**
* Nanos per millisecond.
*/
Expand Down
16 changes: 15 additions & 1 deletion test/jdk/java/time/test/java/time/TestInstant.java
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2015, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -60,13 +60,15 @@
package test.java.time;

import java.time.Instant;
import java.time.temporal.ChronoUnit;

import org.testng.annotations.Test;
import org.testng.annotations.DataProvider;
import static org.testng.Assert.assertEquals;

/**
* Test Instant.
* @bug 8273369
*/
@Test
public class TestInstant extends AbstractTest {
Expand Down Expand Up @@ -96,4 +98,16 @@ public void test_epochMillis(String name, long millis) {
assertEquals(millis, m, name);
}

/**
* Checks whether Instant.until() returning microseconds does not throw
* an ArithmeticException for Instants apart for more than Long.MAX_VALUE
* nanoseconds.
*/
@Test
public void test_microsUntil() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment about the test might be helpful.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Added some comments to the test.

var nanoMax = Instant.EPOCH.plusNanos(Long.MAX_VALUE);
var totalMicros = Instant.EPOCH.until(nanoMax, ChronoUnit.MICROS);
var plusOneMicro = Instant.EPOCH.until(nanoMax.plusNanos(1000), ChronoUnit.MICROS);
assertEquals(plusOneMicro - totalMicros, 1L);
}
}