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

[Bug] Potential synchronization problem in CoreASTProvider #1151

Open
jdneo opened this issue Jun 16, 2023 · 0 comments
Open

[Bug] Potential synchronization problem in CoreASTProvider #1151

jdneo opened this issue Jun 16, 2023 · 0 comments

Comments

@jdneo
Copy link
Contributor

jdneo commented Jun 16, 2023

I found there might be a potential synchronization issue in CoreASTProvider. Take the following picture as an example:

image

In this picture:

  • the left side represents a thread which requires an AST of current active compilation unit.
  • the right side represents the editor thread.
  • the middle is CoreASTProvider, both above two threads interact with CoreASTProvider.

What happens:

  1. Thread A calls getAST(...). At that time, the cached AST is null, so createAST() is called and the created AST is returned. After that, CoreASTProvider will call cache(...) to cache the AST.
  2. At the moment when AST is being created, document change happens. Editor invokes disposeAST().
  3. Given that getAST() is not synchronized with disposeAST(), though editor invokes disposeAST(), an out-of-date AST is still cached. Which means, other threads which call getAST(...) after that moment, they will get an out-of-date AST.
jdneo added a commit to jdneo/eclipse.jdt.ls that referenced this issue Jun 19, 2023
- There are chances that the AST used for semantic highlighting is
  out-of-date, which leads to wrong highlighting. This fix simply compare
  the length of the document with the length of the AST to check if the
  AST is out-of-date. In most of the case, checking the length is enough.

- For root cause analysis, see: eclipse-jdt/eclipse.jdt.core#1151

Signed-off-by: Sheng Chen <[email protected]>
testforstephen pushed a commit to eclipse-jdtls/eclipse.jdt.ls that referenced this issue Jul 20, 2023
…2709)

- There are chances that the AST used for semantic highlighting is
  out-of-date, which leads to wrong highlighting. This fix simply compare
  the length of the document with the length of the AST to check if the
  AST is out-of-date. In most of the case, checking the length is enough.

- For root cause analysis, see: eclipse-jdt/eclipse.jdt.core#1151

Signed-off-by: Sheng Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant