Build: InconsistentAnalysisException: Requested result might be inconsistent with previously returned results

[atreeon]

Hi, I'm getting the following error on build, any ideas please?

I have since upgraded to Dart 2.8.1.

Renaming the files fixes the problem temporarily

https://github.com/dart-lang/sdk/issues/41298

Dart VM version: 2.7.0-dev.2.1 (Mon Dec 2 20:10:59 2019 +0100) on "macos_x64"

Same as here but it was closed without being resolved (despite a few people saying it is an issue)

36618

I cannot run my whole test suite anymore. Unfortunately I can't share my code and I can't seem to recreate it. It happens when I build some generated code.

Any ideas?

[jakemac53]

We are trying to track this down but haven't had any luck so far.

[atreeon]

thanks for having a look @jakemac53 , please excuse me for not knowing even where to start with looking at this error, however, maybe some clues for you:

Intermittently but regularly, if I make a change to a file where code gen is required, then I get the error
I then need to rename the file for the error to go away (I usually put an underscore at the end of the filename)
When I subsequently regenerate my code gen files again (pub run build) then I don't get the error again until I change the file.

[atreeon]

Note the word cached in brackets (deleting the files doesn't help though)
[SEVERE] value_t2_generator:value_t2 on test/ex18_.dart (cached):
Error running ValueT2Generator
InconsistentAnalysisException: Requested result might be inconsistent with previously returned results

[jakemac53]

I have made some traction on this issue, see the repro I added in https://github.com/dart-lang/build/pull/2705.

Essentially, any call to libraryFor or isLibrary may now invalidate the current analysis session - which includes any Elements you already have.

Specifically grabbing the session off of an old Element, and then calling some method on that, is likely to result in this exception.

The workaround is to grab a new LibraryElement - using resolver.libraryFor, and using the session from _that_. You can fairly easily do this for any existing LibraryElement you have, like this:

var newLibraryElement =
      await resolver.libraryFor(await resolver.assetIdForElement(oldLibraryElement));

[jakemac53]

cc @stereotype441 @scheglov for fyi (see my last comment). We might want to chat about this and see if there is some more elegant workaround or fix.

[scheglov]

I don't know what libraryFor and isLibrary do, and why you say that they may invalidate the current session, so cannot estimate whether this a bug in analyzer, or an expected behavior. In general, if you tell AnalysisDriver that a file changed (addFile, changeFile, or removeFile), your previous session gets invalidated.

[jakemac53]

libraryFor and isLibrary both take a dart asset as an argument, and either give you a LibraryElement for it, or return a bool telling you if it is a library at all or not.

If those assets have not been seen before then they can cause calls to addFile or changeFile, which is invalidating the session and causing this problem for people.

We used to simply not allow resolving any files not imported by the primary entrypoint which is why this issue came up recently (we started allowing you to resolve additional files, including ones you just output).

I do understand why this would invalidate the session - the question I had for you all is what the best remedy might be to suggest when people run into this.

[simolus3]

Maybe the analyzer could add some features to make the this easier for builder authors:

1. Something like a bool isInconsistent getter in AnalysisSession that returns _driver.currentSession != this. Then builders would only need to fetch a new session if necessary.
2. Some service to transform elements from an old analysis session to elements of another. I'm not sure if this is feasible, but maybe the analyzer could map old elements by name. So if I have say a MethodElement from an old session I could obtain the equivalent MethodElement from a new session (if a 1:1 mapping exists, otherwise it should throw).

[scheglov]

@jakemac53 Why do we have to call addFile or changeFile? Do some files change while a builder runs, and we want builders to see them? And generally speaking addFile is only for Analysis Server, when you want to subscribe to see files reanalyzed when a dependency changes.

You don't have to do anything special to ask for a LibraryElement, neither addFile, nor changeFile. You just ask for it, and AnalysisDriver will read the file, and its dependencies.

@simolus3 We could add bool isConsistent to AnalysisSession, but I don't understand the use case. If it is the builder that changed a file, then it knows when to ask for the new session. If it is not the builder, then I don't understand what else could change a file.

[natebosch]

Why do we have to call addFile or changeFile? Do some files change while a builder runs, and we want builders to see them?

File contents never change, but new files get written, including those which are already imported from places that we previously analyzed when they were missing.

And generally speaking addFile is only for Analysis Server, when you want to subscribe to see files reanalyzed when a dependency changes.

Hmm, maybe we shouldn't be calling it then? We call it anytime we "write" a new file.

Some confusion, we call updateFile or newFile, doesn't look like we call addFile.
https://github.com/dart-lang/build/blob/70add123514e7f06d91137d4d1bfa6bcc80e56b9/build_resolvers/lib/src/build_asset_uri_resolver.dart#L85-L91

Then for either of those cases we call driver.changeFile

https://github.com/dart-lang/build/blob/70add123514e7f06d91137d4d1bfa6bcc80e56b9/build_resolvers/lib/src/build_asset_uri_resolver.dart#L97

You just ask for it, and AnalysisDriver will read the file, and its dependencies.

We're handling the case where AnalysisDriver already read the file, found it didn't exist, and is using an empty source placeholder for it.

If it is the builder that changed a file, then it knows when to ask for the new session. If it is not the builder, then I don't understand what else could change a file.

The changeFIle call might be in response to trying to analyze a file that was not yet analyzed. When a builder asks for a LibraryElement, if we haven't seen it before we crawl transitive imports for that library and ensure they are all visible to the analysis driver, calling changeFile for any files that are "new" in the sense that the analyzer either hasn't seen them before, or the last time it asked for them they were missing.

We also don't have a lock around this, so different builders _cant_ add files while some other builder is using an analysis sessions, but that isn't necessary to reproduce what we are seeing.

[scheglov]

Thank you for the explanation.

Yes, if we need to create previously not existed, but already read files, then we need to call changeFile.

I can add bool isInconsistent getter to AnalysisSession, it is easy and might be handy in some cases. But is this what we want? Is it convenient for builder authors to use? What would they do if the session is not consistent?

@simolus3 As for requesting corresponding element, I'd like to know which elements you want to request, and why.

[simolus3]

I'd like to know which elements you want to request, and why.

@scheglov For me, this came up when trying to load the ast node defining an element (where element.session might be outdated). However, I noticed that ResolvedLibraryResult.getElementDeclaration works for elements from a different session as well (it seems to just compare the source offsets of the declaration). As long as that continues to work, I don't need to map elements between sessions.

[simolus3]

What would they do if the session is not consistent?

Builder's can always grab a fresh session by using build_resolvers-apis. So right now the logic is to use the session attached to an element, catch the InconsistentAnalysisException, obtain a new session and retry. Being able to check whether a session is inconsistent beforehand would make this a bit easier.

I've opened https://dart-review.googlesource.com/c/sdk/+/152149/ to add the isInconsistent getter.

[jakemac53]

Closing this - I don't think there is anything actionable on this repo at this point. See my comment here https://github.com/dart-lang/build/issues/2634#issuecomment-670603224.

If we want/need a new api in analyzer lets create an issue in the SDK issue tracker for that.