search

oracle / graal

GraalVM compiles Java applications into native executables that start instantly, scale fast, and use fewer compute resources 🚀
https://www.graalvm.org
Other
20.09k stars 1.6k forks source link

ZonedDateTime within a timeline overlap changes the instant it represents when passed through the polyglot boundary #4918

Open radeusgd opened 1 year ago

radeusgd commented 1 year ago

Describe GraalVM and your environment :

Have you verified this issue still happens when using the latest snapshot?

No.

Describe the issue

The issue is that in the InteropLibrary the datetime consists only of a Time, Date and a TimeZone. This is however not always enough and can be a source of ambiguity - to have an unambiguous representation in all cases, the offset is also necessary.

The issue shows during the autumn Daylight Saving Time switch - when the two timelines overlap and times like 2:30am can mean two moments in time. At such moment, the timezone is not enough to unambiguously pinpoint the time instant - the offset is needed too.

Currently, the InteropLibrary only allows to expose the Date, Time and TimeZone, so there's no way for a representation of a date-time in a Truffle language to also expose the zone. Thus if a ZonedDateTime representing such an ambiguous location in time is passed to such language and then converted back to Java, it necessarily loses information.

See the HostToTypeNode::asJavaObject method, HostToTypeNode.java:464:

else if (targetType == ZonedDateTime.class) {
    if (interop.isDate(value) && interop.isTime(value) && interop.isTimeZone(value)) {
        LocalDate date;
        LocalTime time;
        ZoneId timeZone;
        try {
            date = interop.asDate(value);
            time = interop.asTime(value);
            timeZone = interop.asTimeZone(value);
        } catch (UnsupportedMessageException e) {
            throw shouldNotReachHere(e);
        }
        obj = createZonedDateTime(date, time, timeZone);
    }

...

@TruffleBoundary
private static ZonedDateTime createZonedDateTime(LocalDate date, LocalTime time, ZoneId timeZone) {
    return ZonedDateTime.of(date, time, timeZone);
}

Note that when using ZonedDateTime.of(date, time, timeZone) we lose the offset information. To unambiguously migrate such instant in time, the ZonedDateTime.ofLocal(datetime, timezone, offset) should be used.

Thus, I would like to suggest if it is possible to add ZoneOffset support to the polyglot mechanism, so that datetimes can register to also expose the offset and allow it to be used when reconstructing the ZonedDateTime.

I don't have an exact repro, as I couldn't quickly find a way to handle a Java date on the side of JS, we are encountering this issue in Enso, for example see the following test suite.

To make the issue a bit more clear, I can show the following +- steps of action:

var polyglotMethod; // assume a polyglot method that takes a ZonedDateTime, converts it into a polyglot value of some guest language and then converts it back into ZonedDateTime

var tz = ZoneId.of("Europe/Warsaw");

// 2:30 am before DST switch: 2022-10-30T02:30:00+02:00[Europe/Warsaw]
var dt1 = ZonedDateTime.of(2022, 10, 30, 2, 30, 0, 0, tz);

// 2:30 am after DST switch (note the offset changed to +01:00): 2022-10-30T02:30+01:00[Europe/Warsaw]
var dt2 = dt1.plusHours(1);

// Note how these will differ by 3600.
System.out.println(dt1.toEpochSeconds());
System.out.println(dt2.toEpochSeconds());

var dt1Prime = polyglotMethod.execute(dt1);
var dt2Prime = polyglotMethod.execute(dt2);

// And afterwards, both datetimes will point at the instant in time associated with `dt1`. The distinction of `dt2` has been lost.
System.out.println(dt1Prime.toEpochSeconds());
System.out.println(dt2Prime.toEpochSeconds());
oubidar-Abderrahim commented 1 year ago

Thank you for reporting this, we'll take a look into it shortly

radeusgd commented 1 year ago

Thanks, it will be appreciated.

I was trying to provide you with a better repro, on build 22.3.0-dev, but it is a bit non-trivial - I cannot rely on JS or Python as these languages do not handle the timeline overlap correctly. It seems of the by-default-available languages, Espresso may be the best bet - as it has the same semantics as regular Java so the ZonedDateTime itself will behave correctly with timeline overlap and should be able to show the issue when crossing the polyglot boundary.

I've tried the following code:

package org.enso.base.repro;

import java.time.ZonedDateTime;
import java.util.function.Function;

public class EspressoSide {
    public static void hello() {
        System.out.println("Hello from Espresso!");
    }

    public static void handleDate(Function<ZonedDateTime, Long> proxy, ZonedDateTime date) {
        System.out.println("Espresso: " + date + " @ " + date.toEpochSecond());
        long res = proxy.apply(date);
        System.out.println("Got back: " + res);
    }
}
package org.enso.base.repro;

import org.graalvm.polyglot.Context;

import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.function.Function;

public class MainApplication {
  public static void main(String[] args) {
    Context ctx =
        Context.newBuilder()
            .option("java.Classpath", "/home/radeusgd/NBO/repro_dst/bar")
            .option("java.PolyglotInterfaceMappings", "java.util.function.Function")
            .allowAllAccess(true)
            .build();
    var espresso = ctx.getBindings("java").getMember("org.enso.base.repro.EspressoSide");
    System.out.println(espresso.invokeMember("hello"));

    Function<ZonedDateTime, Long> proxy = date -> {
      long x = date.toEpochSecond();
      System.out.println("JVM: " + date + " @ " + x);
      return x;
    };

    ZoneId tz = ZoneId.of("Europe/Warsaw");
    ZonedDateTime date1 = ZonedDateTime.of(2022, 10, 30, 2, 30, 0, 0, tz);
    ZonedDateTime date2 = date1.plusHours(1);

    System.out.println(date1);
    espresso.invokeMember("handleDate", proxy, date1);

    System.out.println();
    // I expect the date in the callback to be shifted and fall back to `date1`.
    System.out.println(date2);
    espresso.invokeMember("handleDate", proxy, date2);
  }
}

Unfortuantely I cannot get it to work:

Hello from Espresso!
NULL
2022-10-30T02:30+02:00[Europe/Warsaw]
Exception in thread "main" java.lang.IllegalArgumentException: Invalid argument when invoking 'handleDate' on 'Klass<Lorg/enso/base/repro/EspressoSide;>'(language: Java, type: com.oracle.truffle.polyglot.PolyglotMapAndFunction). Could not cast foreign object to java/time/ZonedDateTime: due to: Missing field: dateTimeProvided arguments: ['2022-10-30T02:30+02:00[Europe/Warsaw]'(language: Java, type: java.time.ZonedDateTime)].
    at org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotEngineException.illegalArgument(PolyglotEngineException.java:131)
    at org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotValueDispatch.invalidInvokeArgumentType(PolyglotValueDispatch.java:1432)
(...)

For some reason the conversion to ZonedDateTime seems to fail altogether. Do you have some ideas how I might fix it? Anyway I hope the repro sketch maybe will be helpful as a starting point.

I reported the issue blocking this repro from working as a separate issue. Once that is resolved, we can try creating an improved repro for this one. Alternatively, one could try extending SimpleLanguage with proper Java-based date interop and show the issue on that.