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

JavaSymbolResolver used in parallel #2668

Closed
corradomio opened this issue May 14, 2020 · 1 comment · Fixed by #2694
Closed

JavaSymbolResolver used in parallel #2668

corradomio opened this issue May 14, 2020 · 1 comment · Fixed by #2694
Labels
Improvement Not a bug, but a way that JP can be be enhanced to work better.
Milestone

Comments

@corradomio
Copy link

In this moment, it is not possible to use JavaSymbolResolver in parallel.
The problem is in JavaParserFacade, in the static method get:

public static JavaParserFacade get(TypeSolver typeSolver) {
    return instances.computeIfAbsent(typeSolver, JavaParserFacade::new);
}

instances is a WeakHashMap but this class is not thread-safe.
It can be enough to convert this method in a "synchronized" method:

public static **synchronized** JavaParserFacade get(TypeSolver typeSolver) {
    return instances.computeIfAbsent(typeSolver, JavaParserFacade::new);
}
@MysterAitch MysterAitch added the Improvement Not a bug, but a way that JP can be be enhanced to work better. label May 14, 2020
@MysterAitch MysterAitch added this to the next release 3.15.23 milestone May 17, 2020
@MysterAitch
Copy link
Member

Just wanted to flag up these really helpful / thoughtful comments from @migliorabile in a separate issue, to give some more context:

#2671 (comment)

#2671 (comment)

At the moment, "JavaParserFacade.get()" IS ""the problem""because

1. it is a GLOBAL/STATIC method

2. it caches ALL type solvers

3. it is used in several places inside the library

4. it is NOT synchronized

I tried to remove it, but I have not implemented this part.
The current ""possible"" solution is

1. to create a JavaParserFacadeFactory that contains the map typesolver -> JavaParserFacade (the current static part of JavaParserFacade  inserted in a separated class)

2. to extend TypeSolver with a method "getFactory" that return a JavaParserFacadeFactory >

The call (repeat, used in the library implementation)

JavaParserFacade.get(typeSolver)

it will converted in

public static JavaParserFacade get(TypeSolver typeSolver) {
    return typesolver.getRoot().getFactory().get(typeSolver)
}

In this way, the root typeSolver contains the ""thread-local"" cache of type solvers, and with a minimum impact on the current code. The problem is to change the current typeSolver interface.

But I am not sure.

MysterAitch added a commit to MysterAitch/javaparser that referenced this issue May 22, 2020
…get`. Note that this is specifically in response to javaparser#2668 and is not indicative of JavaParser being safe to use within a multi-threaded environment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Not a bug, but a way that JP can be be enhanced to work better.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants