error when reading class file with unknown newer jdk version #18618

bishabosha · 2023-09-29T11:20:05Z

when reading a classfile causes a runtime exception, report the version of the classfile, and request that the user checks JDK compatibility.

Considerations:

should we test this?

currently we have no process for testing latest JDK, currently we still only test with JDK 16 and 8

fixes #18573

Release Notes

If there is an error reading a class file, we now report the version of the classfile, and a recommendation to check JDK compatibility:

error while loading String,
  class file /modules/java.base/java/lang/String.class is broken (version 65.0),
  please check the JDK compatibility of your Scala version (3.4.0-RC1-bin-SNAPSHOT-nonbootstrapped),
  reading aborted with class java.lang.RuntimeException:
  foo bar

This will improve the user experience for when the class file format changes in an unexpected way (for example in the JDK 21 release, which is now supported by Scala 3.3.1)

sjrd · 2023-10-04T07:54:56Z

Can we somehow only report that clear error message if there is actually an exception thrown while reading the class file?

What does Scala 2 do in this situation?

bishabosha · 2023-10-04T09:06:20Z

What does Scala 2 do in this situation?

From what I see there is no capturing of the class file version, and all runtime exceptions will be wrapped in the same "class file 'foo' is broken" error as dotty.

@lrytz @SethTisue what do you think?

I also think it would be ideal to not pre-emptively error, but append an addendum to any error that might occur.

bishabosha · 2023-10-04T09:08:41Z

compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala

+        s"class file '${classfile}' has unknown version $majorVersion.$minorVersion, should be at least $JAVA_MAJOR_VERSION.$JAVA_MINOR_VERSION")
+    if majorVersion > JAVA_LATEST_MAJOR_VERSION then
+      throw new IOException(
+        s"class file '${classfile}' has unknown version $majorVersion.$minorVersion, and was compiled by a newer JDK than supported by this Scala version, please update to a newer Scala version.")


We could also put in the error message which is the latest classfile major supported, and should we reverse map that to the corresponding JDK version? (more work for @Kordyjan release procedure)

lrytz · 2023-10-04T12:31:44Z

Can you reproduce the error described in the ticket? It seems to work for me.

➜ sandbox sc3 -version
Scala compiler version 3.3.1 -- Copyright 2002-2023, LAMP/EPFL
➜ sandbox java -version
openjdk version "21" 2023-09-19
OpenJDK Runtime Environment (build 21+35-2513)
OpenJDK 64-Bit Server VM (build 21+35-2513, mixed mode, sharing)
➜ sandbox cat main.scala
import java.nio.file.StandardOpenOption

@main def Test = {
  val x =  StandardOpenOption.CREATE
}
➜ sandbox sc3 main.scala

bishabosha · 2023-10-04T12:58:16Z

Can you reproduce the error described in the ticket? It seems to work for me.

3.3.1 is patched for jdk 21, it should break reliably with 3.3.0, e.g.

$> cs launch org.scala-lang:scala3-compiler_3:3.3.0  --jvm zulu:21 -M dotty.tools.MainGenericCompiler -- -usejavacp foo.scala
error while loading StandardOpenOption,
class file /modules/java.base/java/nio/file/StandardOpenOption.class is broken, reading aborted with class java.lang.RuntimeException
bad constant pool index: 0 at pos: 1200
-- [E008] Not Found Error: foo.scala:4:30 --------------------------------------
4 |  val x =  StandardOpenOption.CREATE
  |           ^^^^^^^^^^^^^^^^^^^^^^^^^
  |   value CREATE is not a member of object java.nio.file.StandardOpenOption
2 errors found

lrytz · 2023-10-04T13:58:18Z

👍 thanks

Older 2.13 crashes also with no user friendly information. In the past years this happened very rarely - maybe it will become more frequent though as the pace of changes in the JVM is increasing. Still, I think it would be better not to have a hard error based on the classfile version and instead provide better message to the user on crashes. We can avoid giving the user an error when it works, which seems to be the common case. Also we don't need to keep updating the version number as Java releases come out.

➜ sandbox scv 2.13.10 A.scala
error:
  bad constant pool index: 0 at pos: 48445
     while compiling: <no file>
        during phase: globalPhase=<no phase>, enteringPhase=<some phase>
     library version: version 2.13.10
    compiler version: version 2.13.10
  reconstructed args:

  last tree to typer: EmptyTree
       tree position: <unknown>
            tree tpe: <notype>
              symbol: null
           call site: <none> in <none>

== Source file context for tree position ==

error: scala.reflect.internal.FatalError:
  bad constant pool index: 0 at pos: 48445
     while compiling: <no file>
        during phase: globalPhase=<no phase>, enteringPhase=<some phase>
     library version: version 2.13.10
    compiler version: version 2.13.10
  reconstructed args:

  last tree to typer: EmptyTree
       tree position: <unknown>
            tree tpe: <notype>
              symbol: null
           call site: <none> in <none>

== Source file context for tree position ==


	at scala.reflect.internal.Reporting.abort(Reporting.scala:69)
	at scala.reflect.internal.Reporting.abort$(Reporting.scala:65)
	at scala.reflect.internal.SymbolTable.abort(SymbolTable.scala:28)
	at scala.tools.nsc.symtab.classfile.ClassfileParser$ConstantPool.errorBadIndex(ClassfileParser.scala:408)
	at scala.tools.nsc.symtab.classfile.ClassfileParser$ConstantPool.getExternalName(ClassfileParser.scala:263)

som-snytt · 2023-10-04T14:25:53Z

This happens to me frequently, during casual testing. Spurious version mismatches, including tasty format of stale files I don't care about during casual testing, are a mild annoyance. Thanks for any mitigation or amelioration.

bishabosha · 2023-10-05T08:47:45Z

Also we don't need to keep updating the version number as Java releases come out

In this case we can still say "class file 'foo' is broken... the class file version was 65.0, please check the JDK compatibility of your Scala version (3.x.y)."

SethTisue

Kordyjan removed the backport:nominated label last week

Curious why? I would have thought this was a strong candidate for backporting.

My observation on Discord and other community forums is that yes, this definitely actually happens to users quite often, and the users are often newcomers trying to run Scala for the first time. These newcomers often simply do not share our veterans’ awareness that it's important to aggressively update compiler and build tool versions if you want to use a recent JDK. It's rather common that their package manager gives them a brand new JDK but the learning materials they are using specify older Scala and/or sbt versions.

As for the PR in general, it looks good to me, and I agree with Lukas's remarks.

bishabosha · 2023-10-17T07:02:45Z

Curious why? I would have thought this was a strong candidate for backporting.

I would guess because it makes a hard error when none would have existed before, so far LTS compatibility is basically "must be identical unless a flag is used"

I think its pretty clear from the comments that instead we should passively track the version, then report it whenever there is the "broken" class file error - which would match the strict LTS compatibility rules

bishabosha · 2023-10-31T17:22:07Z

@SethTisue with the latest change, we only report the JDK version along with the broken version message, this will produce something like:

error while loading String,
  class file /modules/java.base/java/lang/String.class is broken (version 65.0),
  please check the JDK compatibility of your Scala version (3.4.0-RC1-bin-SNAPSHOT-nonbootstrapped),
  reading aborted with class java.lang.RuntimeException:
  foo bar

and with this change it is compatible with LTS

bishabosha · 2023-11-03T11:57:37Z

rebased

…n" to LTS (#20862) Backports #18618 to the LTS branch. PR submitted by the release tooling. [skip ci]

bishabosha requested review from sjrd and Kordyjan September 29, 2023 11:20

bishabosha assigned sjrd Sep 29, 2023

bishabosha requested review from SethTisue and WojciechMazur September 29, 2023 11:21

bishabosha added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Sep 29, 2023

bishabosha commented Oct 4, 2023

View reviewed changes

Kordyjan removed the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Oct 10, 2023

SethTisue reviewed Oct 16, 2023

View reviewed changes

bishabosha requested a review from SethTisue October 31, 2023 17:20

bishabosha added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Oct 31, 2023

sjrd approved these changes Nov 1, 2023

View reviewed changes

SethTisue approved these changes Nov 2, 2023

View reviewed changes

bishabosha added 2 commits November 3, 2023 12:46

error when reading class file with unknown newer jdk version

f430e44

track JDK version only when runtime exception occurs

2183bf9

bishabosha force-pushed the warn-unknown-jdk branch from 0ff2ad3 to 2183bf9 Compare November 3, 2023 11:57

bishabosha enabled auto-merge November 3, 2023 11:57

bishabosha merged commit 8c602b3 into scala:main Nov 3, 2023
18 checks passed

bishabosha deleted the warn-unknown-jdk branch November 3, 2023 13:25

SethTisue mentioned this pull request Nov 4, 2023

Weird error in REPL: bad constant pool index: 0 VirtusLab/scala-cli#2509

Closed

bishabosha added the release-notes Should be mentioned in the release notes label Nov 8, 2023

Kordyjan added this to the 3.4.0 milestone Dec 20, 2023

WojciechMazur mentioned this pull request Jun 28, 2024

Backport "error when reading class file with unknown newer jdk version" to LTS #20862

Merged

WojciechMazur added a commit that referenced this pull request Jul 1, 2024

Backport "error when reading class file with unknown newer jdk versio…

9bdef0b

…n" to LTS (#20862) Backports #18618 to the LTS branch. PR submitted by the release tooling. [skip ci]

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

error when reading class file with unknown newer jdk version #18618

error when reading class file with unknown newer jdk version #18618

bishabosha commented Sep 29, 2023 •

edited

Loading

sjrd commented Oct 4, 2023

bishabosha commented Oct 4, 2023

bishabosha Oct 4, 2023 •

edited

Loading

lrytz commented Oct 4, 2023

bishabosha commented Oct 4, 2023 •

edited

Loading

lrytz commented Oct 4, 2023

som-snytt commented Oct 4, 2023

bishabosha commented Oct 5, 2023 •

edited

Loading

SethTisue left a comment

bishabosha commented Oct 17, 2023 •

edited

Loading

bishabosha commented Oct 31, 2023 •

edited

Loading

bishabosha commented Nov 3, 2023

error when reading class file with unknown newer jdk version #18618

error when reading class file with unknown newer jdk version #18618

Conversation

bishabosha commented Sep 29, 2023 • edited Loading

Release Notes

sjrd commented Oct 4, 2023

bishabosha commented Oct 4, 2023

bishabosha Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

lrytz commented Oct 4, 2023

bishabosha commented Oct 4, 2023 • edited Loading

lrytz commented Oct 4, 2023

som-snytt commented Oct 4, 2023

bishabosha commented Oct 5, 2023 • edited Loading

SethTisue left a comment

Choose a reason for hiding this comment

bishabosha commented Oct 17, 2023 • edited Loading

bishabosha commented Oct 31, 2023 • edited Loading

bishabosha commented Nov 3, 2023

bishabosha commented Sep 29, 2023 •

edited

Loading

bishabosha Oct 4, 2023 •

edited

Loading

bishabosha commented Oct 4, 2023 •

edited

Loading

bishabosha commented Oct 5, 2023 •

edited

Loading

bishabosha commented Oct 17, 2023 •

edited

Loading

bishabosha commented Oct 31, 2023 •

edited

Loading