mbien commented on code in PR #5998:
URL: https://github.com/apache/netbeans/pull/5998#discussion_r1212409120
##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -133,9 +147,23 @@ public Collection<? extends ProjectProblem> getProblems() {
* @return project problems.
*/
Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
- final MavenProject prj =
project.getLookup().lookup(NbMavenProject.class).getMavenProject();
+ return doGetProblems1(sync).first();
+ }
+
+ /**
+ * Analyzes problem, returns list of problems and priming build status.
The returned {@link Pair}
+ * contains the list of problems and true/false whether the priming build
seems necessary. The last result
+ * is cached for the given maven model instance. If the project was
reloaded, the problems will be computed
+ * again for the new project instance. The call might block waiting on the
pending project reload. If `sync'
+ * is false, the method will just post in request processor and return
{@code null}.
+ *
+ * @param sync if the call should complete synchronously
+ */
+ private Pair<Collection<ProjectProblem>, Boolean> doGetProblems1(boolean
sync) {
+ final MavenProject updatedPrj =
((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+ Callable<Pair<Collection<ProjectProblem>, Boolean>> c;
+
synchronized (this) {
- LOG.log(Level.FINER, "Called getProblems for {0}", project);
//lazy adding listener only when someone asks for the problems the
first time
if (projectListenerSet.compareAndSet(false, true)) {
Review Comment:
While looking through this I noticed that `projectListenerSet` is an atomic
primitive but also accessed in a synchronized block. Looks like it can be a
regular `boolean` since i don't think it is used anywhere else.
Same for `problemsCache` which is an `AtomicReference` but could be a
regular reference.
It looks like the `synchronized` blocks were added later without removing
the original concurrency mechanism.
##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -144,59 +172,87 @@ Collection<? extends ProjectProblem>
doGetProblems(boolean sync) {
project.getLookup().lookup(NbMavenProject.class).addPropertyChangeListener(projectListener);
}
-
+ MavenProject o = analysedProject.get();
+ LOG.log(Level.FINER, "Called getProblems for {0}, analysed = {1},
current = {2}",
+ new Object[] { project, o == null ? 0 :
System.identityHashCode(o), System.identityHashCode(updatedPrj) });
//for non changed project models, no need to recalculate, always
return the cached value
- Object wasprocessed =
prj.getContextValue(MavenModelProblemsProvider.class.getName());
- if (wasprocessed != null) {
- Collection<ProjectProblem> cached = problemsCache.get();
- LOG.log(Level.FINER, "Project was processed, cached is: {0}",
cached);
+ Object wasprocessed =
updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+ if (o == updatedPrj && wasprocessed != null) {
+ Pair<Collection<ProjectProblem>, Boolean> cached =
problemsCache.get();
+ LOG.log(Level.FINER, "getProblems: Project was processed,
cached is: {0}", cached);
if (cached != null) {
return cached;
}
}
- Callable<Collection<? extends ProjectProblem>> c = new
Callable<Collection<? extends ProjectProblem>>() {
+
+ SanityBuildAction sba = cachedSanityBuild.get();
+ if (sba != null && sba.getPendingResult() == null) {
+ cachedSanityBuild.clear();
+ }
+ c = new Callable<Pair<Collection<ProjectProblem>, Boolean>>() {
Review Comment:
could be a lambda if you want. Conserves some vertical space :)
##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -144,59 +172,87 @@ Collection<? extends ProjectProblem>
doGetProblems(boolean sync) {
project.getLookup().lookup(NbMavenProject.class).addPropertyChangeListener(projectListener);
}
-
+ MavenProject o = analysedProject.get();
+ LOG.log(Level.FINER, "Called getProblems for {0}, analysed = {1},
current = {2}",
+ new Object[] { project, o == null ? 0 :
System.identityHashCode(o), System.identityHashCode(updatedPrj) });
//for non changed project models, no need to recalculate, always
return the cached value
- Object wasprocessed =
prj.getContextValue(MavenModelProblemsProvider.class.getName());
- if (wasprocessed != null) {
- Collection<ProjectProblem> cached = problemsCache.get();
- LOG.log(Level.FINER, "Project was processed, cached is: {0}",
cached);
+ Object wasprocessed =
updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+ if (o == updatedPrj && wasprocessed != null) {
+ Pair<Collection<ProjectProblem>, Boolean> cached =
problemsCache.get();
+ LOG.log(Level.FINER, "getProblems: Project was processed,
cached is: {0}", cached);
if (cached != null) {
return cached;
}
}
- Callable<Collection<? extends ProjectProblem>> c = new
Callable<Collection<? extends ProjectProblem>>() {
+
+ SanityBuildAction sba = cachedSanityBuild.get();
+ if (sba != null && sba.getPendingResult() == null) {
+ cachedSanityBuild.clear();
+ }
+ c = new Callable<Pair<Collection<ProjectProblem>, Boolean>>() {
@Override
- public Collection<? extends ProjectProblem> call() throws
Exception {
- Object wasprocessed =
prj.getContextValue(MavenModelProblemsProvider.class.getName());
- if (wasprocessed != null) {
- Collection<ProjectProblem> cached =
problemsCache.get();
- LOG.log(Level.FINER, "Project was processed #2, cached
is: {0}", cached);
+ public Pair<Collection<ProjectProblem>, Boolean> call() throws
Exception {
+ // double check, the project may be invalidated during the
time.
+ MavenProject prj =
((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+ Object wasprocessed =
updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+ if (o == updatedPrj && wasprocessed != null) {
+ Pair<Collection<ProjectProblem>, Boolean> cached =
problemsCache.get();
+ LOG.log(Level.FINER, "getProblems: Project was
processed #2, cached is: {0}", cached);
if (cached != null) {
return cached;
}
}
- List<ProjectProblem> toRet = new ArrayList<>();
- MavenExecutionResult res =
MavenProjectCache.getExecutionResult(prj);
- if (res != null && res.hasExceptions()) {
- toRet.addAll(reportExceptions(res));
+ int round = 0;
+ List<ProjectProblem> toRet = null;
+ while (round < 3) {
+ try {
+ synchronized (MavenModelProblemsProvider.this) {
+ sanityBuildStatus = false;
+ toRet = new ArrayList<>();
+ MavenExecutionResult res =
MavenProjectCache.getExecutionResult(prj);
+ if (res != null && res.hasExceptions()) {
+ toRet.addAll(reportExceptions(res));
+ }
+ //#217286 doArtifactChecks can call
FileOwnerQuery and attempt to aquire the project mutex.
+ toRet.addAll(doArtifactChecks(prj));
+ //mark the project model as checked once and
cached
+
prj.setContextValue(MavenModelProblemsProvider.class.getName(), new Object());
+ synchronized(MavenModelProblemsProvider.this) {
Review Comment:
one of the two `synchronized` blocks is redundant, probably the inner one?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
