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

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

Open
radeusgd opened this issue Sep 10, 2022 · 2 comments
Assignees

Comments

@radeusgd
Copy link
Contributor

Describe GraalVM and your environment :

  • GraalVM version or commit id if built from source: 21.3.0
  • CE or EE: CE
  • JDK version: JDK11
  • OS and OS Version: Ubuntu 22.04
  • Architecture: amd64
  • The output of java -Xinternalversion:
OpenJDK 64-Bit Server VM (11.0.13+7-jvmci-21.3-b05) for linux-amd64 JRE (11.0.13+7-jvmci-21.3-b05), built on Oct 16 2021 12:17:09 by "buildslave" with gcc 7.3.0

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
Copy link
Member

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

@radeusgd
Copy link
Contributor Author

radeusgd commented Sep 16, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants