Skip to content

Uncatched case in file existence check of SourceFile #14664

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

Closed
chikei opened this issue Mar 11, 2022 · 6 comments · Fixed by #14698
Closed

Uncatched case in file existence check of SourceFile #14664

chikei opened this issue Mar 11, 2022 · 6 comments · Fixed by #14698
Assignees
Labels
itype:bug stat:needs minimization Needs a self contained minimization
Milestone

Comments

@chikei
Copy link

chikei commented Mar 11, 2022

Compiler version

3.0.0 and 3.1.0

Minimized code

here and here (and I believe later is what occurs in this Ammonite issue) assumes JVM always throw NoSuchFileException when path is not exist, but that's not the case under linux if 1. path start with directory and 2. the directory name exists in filesystem as a file

the code assumed:

$ file test
test: cannot open `test' (No such file or directory)
scala> java.nio.file.Files.newInputStream(java.nio.file.Paths.get("test/test.scala"))
java.nio.file.NoSuchFileException: test/test.scala
  at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:92)
  at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
  at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
  at java.base/sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:219)
  at java.base/java.nio.file.Files.newByteChannel(Files.java:380)
  at java.base/java.nio.file.Files.newByteChannel(Files.java:432)
  at java.base/java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:422)
  at java.base/java.nio.file.Files.newInputStream(Files.java:160)
  ... 28 elided

or

$ mkdir test
$ ./scala
scala> java.nio.file.Files.newInputStream(java.nio.file.Paths.get("test/test.scala"))
java.nio.file.NoSuchFileException: test/test.scala
  at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:92)
  at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
  at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
  at java.base/sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:219)
  at java.base/java.nio.file.Files.newByteChannel(Files.java:380)
  at java.base/java.nio.file.Files.newByteChannel(Files.java:432)
  at java.base/java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:422)
  at java.base/java.nio.file.Files.newInputStream(Files.java:160)
  ... 28 elided

and the uncatched case:

$ rmdir test
$ touch test
scala> java.nio.file.Files.newInputStream(java.nio.file.Paths.get("test/test.scala"))
java.nio.file.FileSystemException: test/test.scala: Not a directory
  at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:100)
  at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
  at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
  at java.base/sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:219)
  at java.base/java.nio.file.Files.newByteChannel(Files.java:380)
  at java.base/java.nio.file.Files.newByteChannel(Files.java:432)
  at java.base/java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:422)
  at java.base/java.nio.file.Files.newInputStream(Files.java:160)
  ... 28 elided
@chikei chikei added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 11, 2022
@prolativ
Copy link
Contributor

@chikei could you provide a minimized example when this actually breaks the compiler without references to an external project?

@prolativ prolativ added stat:needs minimization Needs a self contained minimization and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 14, 2022
@chikei
Copy link
Author

chikei commented Mar 14, 2022

$ rm library
$ bin/scala
Welcome to Scala 3.1.1 (11.0.12, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> import scala._

scala>
$
$ touch library
$ bin/scala
import sWelcome to Scala 3.1.1 (11.0.12, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> import scala._
error while loading Typeable$package$,
library/src/scala/reflect/Typeable.scala: Not a directory
error while loading package$package$,
library/src/scala/compiletime/package.scala: Not a directory
-- Error: ---------------------------------------------------------------------------------------------------------
1 |import scala._
  |^
  |package scala.compiletime does not have a member method summonFrom
error while loading $throws$package$,
library/src/scala/runtime/$throws.scala: Not a directory
error while loading language$,
library/src/scala/runtime/stdLibPatches/language.scala: Not a directory
5 errors found

scala>

@chikei chikei removed their assignment Mar 14, 2022
@chikei
Copy link
Author

chikei commented Mar 16, 2022

Does this mean a scala repl example do not count as minimized example ?

@prolativ
Copy link
Contributor

Sorry, I accidentally assigned you again. However your example still looks like a quite exotic corner case because you're modifying your scala installation in some way and this should not be done by users. Similarly if you renamed lib directory to library your scala installation would stop working and that still wouldn't be a bug

@som-snytt
Copy link
Contributor

I think if you have a local file or dir that breaks the class path, then it's on the user. The scripts for starting the compiler or a REPL from the command line are inherently fragile.

I expected the reproducer to be the thing where there is a local file or dir shadowing a package name such as util. This is an old problem on Scala 2 that dragged on for so long, that I don't remember if it was ever resolved.

There was a Scala 3 issue about how the REPL handles local filesystem. That was fixed.

Here is a current difference between 2/3:

➜  snips mkdir util
➜  snips sdk use scala 3.1.1

Using scala version 3.1.1 in this shell.
➜  snips scala
Welcome to Scala 3.1.1 (17.0.2, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> import util.chaining.given

scala> 42.tap(println)
42
val res0: Int = 42

scala>
➜  snips sdk use scala 2.13.8

Using scala version 2.13.8 in this shell.
➜  snips scala
Welcome to Scala 2.13.8 (OpenJDK 64-Bit Server VM, Java 17.0.2).
Type in expressions for evaluation. Or try :help.

scala> import util.chaining._
                   ^
       error: object chaining is not a member of package util

scala>

I see this is actually totally different. -Ydebug shows the stack trace. This is a tasty problem where it has the class file but is incorrectly attempting to read a source file that is relative to cwd. For some reason, it's happy with empty local dir named "library".

scala> import scala.*
java.nio.file.FileSystemException: library/src/scala/reflect/Typeable.scala: Not a directory
        at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:100)
        at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
        at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
        at java.base/sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:218)
        at java.base/java.nio.file.Files.newByteChannel(Files.java:380)
        at java.base/java.nio.file.Files.newByteChannel(Files.java:432)
        at java.base/java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:422)
        at java.base/java.nio.file.Files.newInputStream(Files.java:160)
        at dotty.tools.io.File.inputStream(File.scala:51)
        at dotty.tools.io.PlainFile.input(PlainFile.scala:68)
        at dotty.tools.io.AbstractFile.toByteArray(AbstractFile.scala:170)
        at dotty.tools.dotc.util.SourceFile$.apply(SourceFile.scala:277)
        at dotty.tools.dotc.core.Contexts$Context.getSource$$anonfun$1(Contexts.scala:276)
        at dotty.tools.dotc.util.GenericHashMap.getOrElseUpdate(GenericHashMap.scala:133)
        at dotty.tools.dotc.core.Contexts$Context.getSource(Contexts.scala:276)
        at dotty.tools.dotc.core.Contexts$Context.getSource(Contexts.scala:282)
        at dotty.tools.dotc.core.Contexts$Context.getSource(Contexts.scala:285)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.sourceChangeContext(TreeUnpickler.scala:1410)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.processPackage(TreeUnpickler.scala:738)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.indexStats(TreeUnpickler.scala:723)
        at dotty.tools.dotc.core.tasty.TreeUnpickler.enter(TreeUnpickler.scala:101)
        at dotty.tools.dotc.core.tasty.DottyUnpickler.enter(DottyUnpickler.scala:55)
        at dotty.tools.dotc.core.classfile.ClassfileParser.unpickleTASTY$1(ClassfileParser.scala:888)
        at dotty.tools.dotc.core.classfile.ClassfileParser.unpickleOrParseInnerClasses(ClassfileParser.scala:956)
        at dotty.tools.dotc.core.classfile.ClassfileParser.parseClass(ClassfileParser.scala:165)
        at dotty.tools.dotc.core.classfile.ClassfileParser.$anonfun$1(ClassfileParser.scala:87)
        at dotty.tools.dotc.core.classfile.ClassfileParser.run(ClassfileParser.scala:82)
        at dotty.tools.dotc.core.ClassfileLoader.load(SymbolLoaders.scala:414)
        at dotty.tools.dotc.core.ClassfileLoader.doComplete(SymbolLoaders.scala:409)
        at dotty.tools.dotc.core.SymbolLoader$$anon$1.doComplete(SymbolLoaders.scala:328)
        at dotty.tools.dotc.core.SymbolLoader.complete(SymbolLoaders.scala:343)

I wonder what is the behavior under a different compilation regime? i.e., sbt.

@som-snytt
Copy link
Contributor

Looking up the stack, I see the OP is correct, SourceFile.apply catches:

  def apply(file: AbstractFile | Null, codec: Codec): SourceFile =
    // see note above re: Files.exists is remarkably slow
    val chars =
    try
      new String(file.toByteArray, codec.charSet).toCharArray
    catch
      case _: java.nio.file.NoSuchFileException => Array[Char]()

Now that I see it, I vaguely remember the ancient note that Files.exists is slow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
itype:bug stat:needs minimization Needs a self contained minimization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants