Building more than one target causes InconsistentAnalysisException

[eernstg]

Working on package reflectable, I performed a pub upgrade which changed versions of build related packages as follows:

  build 1.2.2
> build_config 0.4.2 (was 0.4.1+1)
  build_daemon 2.1.3
> build_resolvers 1.3.3 (was 1.3.1)
> build_runner 1.7.4 (was 1.7.3)
> build_runner_core 4.4.0 (was 4.3.0)
> build_test 0.10.12+1 (was 0.10.12)

The package test_reflectable contains a set of tests that are used to test reflectable. After the above changes they are all failing, for instance:

[SEVERE] reflectable:reflectable on test/invoke_test.dart.vm_test.dart:

InconsistentAnalysisException: Requested result might be inconsistent with previously returned results

However, if I edit the file build.yaml such that it only specifies a generate_for: property matching a single entry point library then code generation succeeds, and the test runs as expected.

This behavior might be associated with AnalysisDriver.changeFile being invoked by the getter BuildStepImpl.inputLibrary that the Builder in reflectable is using to get access to the input library, and subsequently to all other entities from the analyzer.

Did one of the build packages change recently (build_runner?) in such a way that it is not possible to build more than one target? Or in such a way that it is now required to do something extra in the case where multiple targets are given, in order to avoid InconsistentAnalysisException?

Here is a scenario that gives rise to the failures:

> git clone 'https://github.com/dart-lang/test_reflectable.git'
> cd test_reflectable
> cd tool
> make

The make command runs pub get, pub run build_runner build test, as make -n shows.

[jakemac53]

cc @natebosch I think we got another report of this recently as well.

also cc @scheglov for analyzer perspective here.

[scheglov]

From the analyzer POV, when AnalysisDriver.changeFile() is called, this invalidates any existing AnalysisSession(s). If analysis results consistency is necessary, avoid updating files. If analysis results consistency is not necessary, ask for a new analysis session, or call AnalysisDriver (in which case you will get results from a new analysis session).

[natebosch]

Yes, this also came up in gitter, also about package:reflectable

https://gitter.im/dart-lang/build?at=5e555d19fa9f20553b4e7e02

[jakemac53]

Interesting, I wonder why we are only seeing this for reflectable?

[eernstg]

Simon Binder mentioned getResolvedLibraryByElement here: https://gitter.im/dart-lang/build?at=5e56443bef8d646099d1fa06.

So I tried to change the reflectable code generator such that it awaits getResolvedLibraryByElement with each library immediately after getting all libraries with await resolver.libraries.toList() (which occurs at the very beginning), and use only these cached ResolvedLibraryResults during code generation (so getResolvedLibraryByElement is never called again). This does not eliminate the InconsistentAnalysisExceptions.

[atreeon]

any update on the InconsistentAnalysisException problem @eernstg? Would rolling back to an older version of source_gen or build work, if so what version could I try? My CI keeps failing! Thank you

[atreeon]

one strange thing is that if I rename the file then it sometimes works again, the exception mentions the word cache in the output so possibly there might be a way of clearing something to get it to work???

[eernstg]

@atreeon wrote:

any update on the InconsistentAnalysisException problem

No. I suspect that it is caused by the build framework (perhaps the build_runner) running many code generation steps in an overlapping fashion (because it's asynchronous). The exception is then thrown when AnalysisDriver.changeFile has been invoked as part of the startup sequence for one code generation operation, and some other code generation operation is then blamed for using data which was obtained before that invocation of changeFile.

However, I've performed experiments where the reflectable code generator performs all invocations of getResolvedLibraryByElement at the very beginning (such that this special method which may cause a file to be re-read from disk is never invoked later during code generation), but that makes no difference. The reflectable code generator has no static members and no global variables are used, so each code generation step is exclusively working on data which has been obtained from the analyzer during that same code generation step.

The only thing that really works is to force code generation to run with exactly one target (one "program").

The latest version of reflectable before the one that needed to introduce all these asynchronous operations was v2.0.12. However, that version depends on analyzer version <0.35.0, so I'm afraid that won't fit in with today's projects.

[atreeon]

ah ok, thank you for the reply, yup, v0.35 won't work! I don't understand the internal workings of the code generator but renaming the files seems to overcome the problem...temporarily (but often it reappears when a new file is added or an existing file is changed).

[eernstg]

@atreeon wrote:

renaming the files seems to overcome the problem

OK, then I'd like to know: Which files are being renamed? Are you generating code for a single entry point, that is, does the end result contain exactly one file named *.reflectable.dart, or do you generate multiple such files? It does sound like you are seeing some behaviors that aren't exactly identical to the ones that I can see, and that might bring up something new.

[atreeon]

so I have something like lecture.dart which produces a lecture.g.dart file. So yes, I have one attribute against my lecture.dart file and it produces just one generated file. If I rename the lecture.dart file then sometimes the InconsistentAnalysisException error message goes away.

This is the error I get, and if I rename ex18_.dart it sometimes works again (it is not always that file though).

[SEVERE] value_t2_generator:value_t2 on test/ex18_.dart:
Error running ValueT2Generator
InconsistentAnalysisException: Requested result might be inconsistent with previously returned results

[jakemac53]

It looks like we never updated this issue - I am going to close it because there is nothing that we can do from our end, but I will describe the problem and workaround:

The problem

Calls to libraryFor and isLibrary can now invalidate the current session, because they may add new sources, which requires calling changeFile for those sources. Whenever changeFile is called, the current analyzer session is invalidated.

The workaround

Any direct usage of an AnalysisSession must make sure it grabs the latest session. If grabbing the session off of a LibraryElement, you will need to first ensure you have the latest library element, something like the following:

// where `resolver` came from something like `var resolver = await buildStep.resolver;`,
// and `existingLibraryElement` is some library element that you have and want to grab
// a session from.
var newLibraryElement =
      await resolver.libraryFor(await resolver.assetIdForElement(existingLibraryElement));
var session = newLibraryElement.session;

This is unfortunately verbose and slow but we don't have other solutions at this time. Note that in between any async calls that session could theoretically be invalidated, so you may even want to add some retry logic.

Generally, avoid using the analysis session at all if you can, and if you do use it you will have to be aware of these issues.

[rrousselGit]

👋
I am currently facing this issue.

Is there an easy way to know what is causing the invalidation? I am not calling libraryFor/isLibrary directly and my entire generator is synchronous, so I am very confused as to why I get this exception.

[natebosch]

and my entire generator is synchronous, so I am very confused as to why I get this exception.

That's pretty interesting. How reliably can you reproduce this?

What version of build_resolvers do you have? If it's an older one I do wonder whether #2866 makes any difference.

[rrousselGit]

How reliably can you reproduce this?

Locally I am unable to reproduce it, but the CI fails relatively often https://github.com/rrousselGit/river_pod/runs/1366244210?check_suite_focus=true

And many users of the code-generator have complained about this exception.

What version of build_resolvers do you have?

Latest, so 1.4.3

[rrousselGit]

I think I have pinpointed the issue.
It appears to be caused by calls to session.getParsedLibraryByElement

Is it expected? By understanding is that this method returns the already parse result, rather than parsing it again. So I am not sure why that would invalidate the analysis

I don't fully understand why I do not have this exception locally either. Since the code is synchronous, there shouldn't be concurrency issues.

[scheglov]

The method is called get, but actually it does parse files from their content on request.

Regardless of what the method does, InconsistentAnalysisException is thrown when somewhere, some code called AnalysisDriver.changeFile or addFile. Not necessary yours code. The exception is a signal that this happened, and new results that would be returned from the session might be not consistent with previously returned results. For example if a.dart imports b.dart, you asked for a.dart element model, the result will have elements from a.dart and b.dart. You you then change b.dart and ask for b.dart elements through the same session, you would get element model of b.dart that if potentially different than the one you got via a.dart element model.

[rrousselGit]

In that case, is there a way to access the AstNode of an Element without invalidating the previously obtained results in the process?

The only reason I have this exception is because I do:

AstNode astNodeForElement(Element element) {
  return element.session
      .getParsedLibraryByElement(element.library)
      ?.getElementDeclaration(element)?.node;
}

But this line forces me to use the workaround after every call to this function, which I run in a loop, so it's very unideal