Skip to content
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

Add global analysis driver pool to protect usages #3313

Merged
merged 8 commits into from May 26, 2022
Merged

Conversation

jakemac53
Copy link
Contributor

@jakemac53 jakemac53 commented May 26, 2022

Fixes #3312

If the driver is used in between calls to changeFile and before applyPendingChanges completes, then you can get InconsistentAnalysisErrors. This change ensures we won't do that through a new single resource pool shared by all users of the driver.

  • Guard all accesses to the driver and session with the new resource.
    • During the resolve, this only grabs the resource once it is fully ready for all of the changeFile calls and subsequent call to applyPendingFileChanges. This prevents deadlocks compared to a global resolution pool, which may call out to package:build apis which can deadlock.
    • This shouldn't cause significant slowdown, most of the code using the pool has a fixed number of async actions it waits on, and none of those actions might need to wait on a build step to complete (they are all async calls to analyzer apis, not build apis).
  • Keeps in place the per action resolve pool, still needed for managing updates to _entryPoints.
  • Temporarily overrides all packages to run with this version of build_resolvers, will revert that commit before landing

Copy link
Member

@natebosch natebosch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we expand the commit message with 2 things:

  • More details about why we need to guard access to reads from the driver.
  • A rough sketch of the way the code is changing - in particular the nested function argument used to lock only part of the work.

build_resolvers/lib/src/resolver.dart Outdated Show resolved Hide resolved
@jakemac53
Copy link
Contributor Author

Can we expand the commit message with 2 things:

How does the updated text look?

@natebosch
Copy link
Member

How does the updated text look?

Looks great to me. Thanks!

@jakemac53 jakemac53 merged commit 7a384f3 into master May 26, 2022
@jakemac53 jakemac53 deleted the driver-pool branch May 26, 2022 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants