scalameta / metals-feature-requests

Issue tracker for Metals feature requests
38 stars 5 forks source link

Running code should take defensive copy of the compiled output #416

Open harpocrates opened 2 months ago

harpocrates commented 2 months ago

Describe the bug

I was a bit surprised to learn that running through both launch configurations and code lens may be running code that was compiled after the run was kicked off.

Repro

Take this code:

package example

object Main extends App {
  val _ = scala.io.StdIn.readLine("waiting until enter...")
  println(s"Helper.four = ${Helper.four}")
}

object Helper {
  def four = 4
}
  1. compile
  2. run with the code lens on Main. Terminal should open with waiting until enter..., but don't hit enter yet!
  3. modify def four = 4 to def four = 5
  4. now hit enter in the terminal
  5. see Helper.four = 5 in the terminal output

Delayed classloading

Same occurs with a launch config, though passing "jvmOptions": ["-verbose:class"], there makes the problem apparent. Here's the terminal output (repeating above steps but with step 2 being "Run > Run Without Debugging"):

...
[0.158s][info][class,load] java.nio.CharBuffer source: shared objects file
[0.158s][info][class,load] java.nio.HeapCharBuffer source: shared objects file
[0.158s][info][class,load] java.nio.charset.CoderResult source: shared objects file
waiting until enter...
[6.227s][info][class,load] example.Helper$ source: file:/Users/alec/Scratch/metals-testing/base-package/.bloop/root/bloop-bsp-clients-classes/classes-Metals-S8GNEqpeT32D6dPwHfD0QQ==/
Helper.four = 5
[6.228s][info][class,load] scala.runtime.BoxedUnit source: file:/Users/alec/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.12.17/scala-library-2.12.17.jar
[6.228s][info][class,load] java.util.IdentityHashMap$IdentityHashMapIterator source: shared objects file
[6.228s][info][class,load] java.util.IdentityHashMap$KeyIterator source: shared objects file
[6.228s][info][class,load] java.lang.Shutdown source: shared objects file
[6.228s][info][class,load] java.lang.Shutdown$Lock source: shared objects file
 *  Terminal will be reused by tasks, press any key to close it. 

Helper doesn't get loaded in until after we hit enter, and it is loaded straight from where bloop compiled it.

Expected behavior

I'd expect that running takes a defensive copy or the classfiles being run (or at least there's a setting to recover that behavior) at the time that I click the code lens or launch the run config.

Operating system

macOS

Editor/Extension

VS Code

Version of Metals

v1.4.2

Extra context or search terms

No response

tgodzik commented 2 months ago

Thanks for reporting! I think we can make it work after https://github.com/scalameta/metals/pull/7006 as otherwise run would become much slower.

tgodzik commented 1 month ago

Ach, actually I misread, since this also happens in debug mode. The issue is that we compile things in the background.

Copying everything might introduce additional delay, so not sure if we want to do it right now. Will move it to feature requests for now

harpocrates commented 1 week ago

Could we at least introduce a flag around this?

I have users who are hitting an annoying variation of this (happy to open a separate issue for the other as well): they click on the code lens before compilation has finished (or worse - when compilation is failing due to errors) and the lens silently runs old code.

The flag I'm thinking of would enable a slower but more correct code lens:

tgodzik commented 1 week ago

Sure, we could add something like: compilation failed, do you want to run the old code? That would make the users aware of the issue.

As for copying the problem I don't think that's really solvable, at least that way. For large projects we would need to copy every module we depend on, which would be a huge amount of copying. It would also need to be done in Bloop, metals and any other bsp server. I need to think about how to do that properly and in a way that's maintainable.