-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
App-JIT failure for reload sources test after #39869
Comments
This particular issue is fixed with https://dart-review.googlesource.com/c/sdk/+/130374/. A more general problem persists through (and wasn't caused by my CL): Say you have this file:
and you make a app-jit snapshot of it:
and remove it or move it so the path no longer exists:
and run the snapshot in observatory:
and go to observatory, navigates to the isolate and clicks the
Again, basically the same thing happens (I'm guessing): I'll re-assign this issue to @mkustermann who can probably assign a VM engineer to fix that underlying issue. |
We might simply want to disable using hot-reload for isolates which were started from AppJIT snapshots. This would also make our work on the concurrency easier. @rmacnak-google wdyt? |
After cl/128585 landed a app-jit failure started throwing an error: >testee>err> kernel-service: Error: Unhandled exception: >testee>err> Bad state: No element >testee>err> #0 Iterable.first (dart:core/iterable.dart:520:7) >testee>err> #1 MappedIterable.first (dart:_internal/iterable.dart:374:31) >testee>err> #2 lookupOrBuildNewIncrementalCompiler (file:///b/s/w/ir/cache/builder/sdk/pkg/vm/bin/kernel_service.dart:400:45) >testee>err> #3 _processLoadRequest (file:///b/s/w/ir/cache/builder/sdk/pkg/vm/bin/kernel_service.dart:679:22) >testee>err> #4 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:174:12) >testee>err> This is caused by the change ultimately adding scripts for the used mixed in files in "LoadedScripts". In this case "dart:collection/map.dart" (as I recall) was added to it. When reloading the source, it runs through all libraries that "isn't dart scheme", and for all of those libraries through all scripts reported by LoadedScripts for that library (now including something from the platform), and checking if it has been modified. Checking if it is modified only works for file:/// uris through, which platform files aren't, so it just reports true (i.e. the file is modified). This is then passed to the kernel service which - based on the list of one file modified - somehow concludes that it has a compiler already (which it doesn't) and then crashes. This CL fixes this specific issue by also skipping scripts that are "dart scheme". There's still an underlying issue that has nothing to do with cl/128585 though. I'll comment on that in the bug (#39869). Bug: 39869 Change-Id: I1a3f2de888ec53c40f4b6b46a369595abae5bb44 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/130374 Commit-Queue: Jens Johansen <jensj@google.com> Reviewed-by: Martin Kustermann <kustermann@google.com>
Is it done? |
@franklinyow What this issue discusses is not used by any customer AFAIK, so it's low priority in general. The original issue was fixed, but Jens pointed out another/similar issue. So we won't close the issue yet. Maybe we'll decide to just disable hot-reload if running in AppJIT mode. |
Moving to D29 |
Removing milestone. It doesn't affect customers afaik and is low priority. |
After cl/128585 landed a app-jit failure started:
The kernel service seems to fail while trying to build a new incremental compiler.
@jensjoha Could you take a look (since it seemingly started after your change)?
The text was updated successfully, but these errors were encountered: