From 9db75815c63b2da213760829123f76cda813635a Mon Sep 17 00:00:00 2001 From: Svata Dedic Date: Fri, 6 Sep 2024 23:40:34 +0200 Subject: [PATCH] Improving re-loading support if the project keeps changing. Added tests. --- .../project/dependency/ProjectReload.java | 4 +- .../reload/ProjectReloadInternal.java | 17 +- .../project/dependency/reload/Reloader.java | 466 +++++++++++------- .../dependency/reload/StateDataListener.java | 26 +- .../spi/ProjectReloadImplementation.java | 42 +- .../MockProjectReloadImplementation.java | 4 + .../reload/ProjectReloadImplTest.java | 90 +++- .../reload/GradleReloadImplementation.java | 13 +- .../GradleReloadImplementationTest.java | 197 ++++++++ .../protocol/TextDocumentServiceImpl.java | 4 +- .../MavenPrimingReloadImplementation.java | 1 - .../queries/MavenReloadImplementation.java | 14 +- .../MavenReloadImplementationTest.java | 250 ++++++++++ 13 files changed, 915 insertions(+), 213 deletions(-) diff --git a/ide/project.dependency/src/org/netbeans/modules/project/dependency/ProjectReload.java b/ide/project.dependency/src/org/netbeans/modules/project/dependency/ProjectReload.java index e40fada36fb8..45574a8deaae 100644 --- a/ide/project.dependency/src/org/netbeans/modules/project/dependency/ProjectReload.java +++ b/ide/project.dependency/src/org/netbeans/modules/project/dependency/ProjectReload.java @@ -965,7 +965,9 @@ public static CompletableFuture withProjectState(Project p, final // special case if the state matches & is consistent: if the attempted quality is LESS // than this request's target, the ReloadImplementation might give up if (!doReload && lastKnown.target.isAtLeast(stateRequest.getTargetQuality())) { - LOG.log(Level.FINE, "FINISHED: Reload {0}, request: {1}, state {2} - NOOP, finished", new Object[] { p, stateRequest, lastKnown.toString() }); + if (LOG.isLoggable(Level.FINE)) { + LOG.log(Level.FINE, "FINISHED: Reload {0}, request: {1}, state {2} - NOOP, finished", new Object[] { p, stateRequest, lastKnown.toString() }); + } return CompletableFuture.completedFuture(lastKnown); } String reason = stateRequest.getReason(); diff --git a/ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/ProjectReloadInternal.java b/ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/ProjectReloadInternal.java index 520eecc74f44..44fd63a8a467 100644 --- a/ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/ProjectReloadInternal.java +++ b/ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/ProjectReloadInternal.java @@ -261,6 +261,8 @@ public StatePartsImpl(Map, ? extends Pr } } + public static final StateParts EMPTY_PARTS = new StatePartsImpl(); + static Collection variantKey(Project p, StateParts parts, Lookup context) { Collection c = new HashList(); c.add(p); @@ -383,7 +385,7 @@ boolean mergeStates(ProjectReload.ProjectState cached, ProjectReload.ProjectStat } Pair createState(ProjectReload.ProjectState previous, Project p, Collection variant, StateParts parts, - boolean force, StateRequest req) { + boolean rejectInconsistent, StateRequest req) { ProjectReload.ProjectState state = doCreateState(p, parts, req); ProjectReload.ProjectState cur = null; @@ -416,6 +418,9 @@ Pair createState(ProjectReload.ProjectState previous, Pr if (!parts.isEmpty()) { l.init(); } + if (!state.isConsistent() && rejectInconsistent) { + return Pair.of(null, state); + } if (cur != previous && previous != null) { if (LOG.isLoggable(Level.FINE)) { LOG.log(Level.FINE, "Project {0}: obsolete previous state {1}", new Object[] { p, previous.toString() }); @@ -526,7 +531,7 @@ public Pair getProjectState0(Project p, Lookup context, } } - StateRef ref; + StateRef ref = null; // try to keep alive for the length of the operation. ProjectState oldS = null; @@ -556,11 +561,9 @@ public Pair getProjectState0(Project p, Lookup context, return Pair.of(ref, none); } } finally { - if (oldS != null) { - // some harmless thing that the optimizer cannot throw away and GC oldS prematurely. - synchronized (oldS) { - oldS.notify(); - } + if (oldS != null && ref.toDetach != null) { + // assume ref must not be null + ref.toDetach.checkFileTimestamps(); } endOperation(p, null, null); } diff --git a/ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/Reloader.java b/ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/Reloader.java index 02e4fb756ec8..61fdb6b20031 100644 --- a/ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/Reloader.java +++ b/ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/Reloader.java @@ -21,8 +21,10 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.concurrent.CancellationException; @@ -33,6 +35,8 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; import javax.swing.event.ChangeEvent; import org.netbeans.api.project.Project; import org.netbeans.api.project.ProjectUtils; @@ -40,10 +44,12 @@ import org.netbeans.modules.project.dependency.ProjectReload; import org.netbeans.modules.project.dependency.ProjectReload.ProjectState; import org.netbeans.modules.project.dependency.ProjectReload.StateRequest; +import org.netbeans.modules.project.dependency.reload.ProjectReloadInternal.StateParts; import org.netbeans.modules.project.dependency.spi.ProjectReloadImplementation; import org.netbeans.modules.project.dependency.spi.ProjectReloadImplementation.ExtendedQuery; import org.netbeans.modules.project.dependency.spi.ProjectReloadImplementation.LoadContext; import org.netbeans.modules.project.dependency.spi.ProjectReloadImplementation.ProjectStateData; +import org.openide.cookies.SaveCookie; import org.openide.filesystems.FileObject; import org.openide.util.Cancellable; import org.openide.util.Lookup; @@ -114,7 +120,15 @@ public final class Reloader { private int reloadRound = 1; private Iterator implIter; + /** + * Implementations that asked for reload last time. + */ private Collection lastRetries = Collections.emptyList(); + + /** + * Implementations that provided inconsistent data last time. + */ + private Collection lastInconsistent = Collections.emptyList(); private Collection, ProjectReloadImplementation.ProjectStateData>> reportedStates = new ArrayList<>(); public Reloader(Project p, StateRequest request, ProjectReloadInternal.StateRef currentRef, Collection impls, final ProjectReloadInternal registry, Throwable origin) { @@ -125,28 +139,7 @@ public Reloader(Project p, StateRequest request, ProjectReloadInternal.StateRef this.originalState = currentRef == null ? null : currentRef.get(); this.forced = request.isForceReload(); this.variantKey = currentRef == null ? null : currentRef.variantKey; - /* - this.completePending.exceptionally(t -> { - if (t instanceof CompletionException) { - t = t.getCause(); - } - if (t instanceof CancellationException) { - CompletableFuture f; - synchronized (this) { - if (ownCompleted) { - return null; - } - cancelled = t; - f = currentStage; - } - if (f != null) { - f.cancel(true); - } - } - // does not matter, we do not allow anyone to chain. - return null; - }); - */ + for (ProjectReloadImplementation impl : impls) { loadData.add(new LoadContextImpl(impl, originalState)); } @@ -243,6 +236,12 @@ public class LoadContextImpl { * Error reported from the implementation */ volatile Throwable reloadError; + + /** + * True, if the load failure should be interpreted as ERROR, rather than BROKEN project. + */ + volatile boolean reportLoadError; + /** * Partial state requested by the implementation, lazy initialized */ @@ -267,12 +266,11 @@ public ProjectReload.StateRequest getRequest() { } public void reinit(ProjectReloadInternal.StateParts parts) { - ReloadSpiAccessor.get().clear(clientContext); - this.clientContext = null; this.parts = parts; this.partialState = null; this.reloadRequested = false; this.reloadError = null; + this.reportLoadError = false; this.inconsistencies = null; this.cancel = null; this.cancellable = null; @@ -282,6 +280,10 @@ public void reinit(ProjectReloadInternal.StateParts parts) { public CancellationException getCancelled() { return cancel; } + + public T getLoadContext() { + return (T)contextData; + } public void setCancellable(Cancellable c) throws CancellationException { Throwable t; @@ -383,163 +385,257 @@ private void markInconsistentParts() { if (inc != null) { LOG.log(Level.FINE, "{0}: loader reports inconsistencies", new Object[]{this, inc}); } - if (d != null) { - Set inc2 = ReloadSpiAccessor.get().getInconsistencies(d); - if (inc != null) { - if (inc2 != null) { - inc.addAll(inc2); - } - LOG.log(Level.FINE, "{0}: part {1} reports inconsistencies", new Object[]{this, inc2}); - } else { - inc = inc2; + if (markInconsistencies(d, inc, parts, this)) { + ctx.reloadRequested = true; + } + } + } + + public static boolean markInconsistencies(ProjectStateData d, Collection inc, StateParts parts, Object logMe) { + boolean requested = false; + if (d != null) { + Set inc2 = ReloadSpiAccessor.get().getInconsistencies(d); + if (inc != null) { + if (inc2 != null) { + inc.addAll(inc2); } + LOG.log(Level.FINE, "{0}: part {1} reports inconsistencies", new Object[]{logMe, inc2}); + } else { + inc = inc2; } - if (inc != null && !inc.isEmpty()) { - for (Class c : inc) { - Lookup.Template t = new Lookup.Template<>(c); - for (ProjectReloadImplementation.ProjectStateData d2 : parts.values()) { - if (d2 == null) { - continue; - } - if (d2.isConsistent() && d2.isValid()) { - if (c.isInstance(d2.getProjectData()) || (d2.getLookup() != null && d2.getLookup().lookupItem(t) != null)) { - if (LOG.isLoggable(Level.FINE)) { - LOG.log(Level.FINE, "{0}: part {1} provides {2}, mark inconsistent and reload", new Object[]{this, d2.toString(), c}); - } - ctx.reloadRequested = true; - d2.fireChanged(false, true); + } + if (inc != null && !inc.isEmpty()) { + for (Class c : inc) { + Lookup.Template t = new Lookup.Template<>(c); + for (ProjectReloadImplementation.ProjectStateData d2 : parts.values()) { + if (d2 == null) { + continue; + } + if (d2.isConsistent() && d2.isValid()) { + if (c.isInstance(d2.getProjectData()) || (d2.getLookup() != null && d2.getLookup().lookupItem(t) != null)) { + if (LOG.isLoggable(Level.FINE)) { + LOG.log(Level.FINE, "{0}: part {1} provides {2}, mark inconsistent and reload", new Object[]{logMe, d2.toString(), c}); } + requested = true; + d2.fireChanged(false, true); } } } } } + return requested; } - - @NbBundle.Messages(value = {"# {0} - project name", "# {1} - list of providers in loop", "ERR_ReloadingLoop=Project is reloading repeatedly. See the log for more details.", "# {0} - project name", "ERR_ProjectModifiedWhileLoading=Project {0} has been modified while reloading", "# {0} - project name", "ERR_ProjectQualityLow=Project {0} could not be loaded."}) - private synchronized CompletableFuture finishLoadingRound() { + + /** + * Checks and makes a retry load. If it does, it returns a Future, otherwise (the current load may terminate) return {@code null}. + * It protects against repeated loads: if the same files are modified, or the same implementations request reload as the last time, the + * operation fails with OUT_OF_SYNC (file inconsistencies) or + * @param variant + * @return + */ + private CompletableFuture maybeMakeRetry(Collection variant) { + // special case: if some of the phases reported ERROR = code error, bail out and do not even try to retry + if (loadData.stream().anyMatch(ctx -> ctx.reportLoadError)) { + return null; + } Collection retries; boolean forceReload = loadData.stream().anyMatch(d -> d.reloadRequested); - markInconsistentParts(); retries = loadData.stream().filter(d -> d.reloadRequested).collect(Collectors.toList()); - Throwable error = null; + Collection fileInconsistencies = new HashSet<>(); + checkFileTimestamps(fileInconsistencies); + boolean inconsistentRetry = request.isConsistent() && !fileInconsistencies.isEmpty(); + if (inconsistentRetry) { + if (!lastInconsistent.isEmpty() && fileInconsistencies.containsAll(lastInconsistent)) { + ProjectOperationException ex = new ProjectOperationException(project, ProjectOperationException.State.OUT_OF_SYNC, Bundle.ERR_ProjectModifiedWhileLoading(ProjectUtils.getInformation(project).getDisplayName())); + registry.createState(originalState, project, variant, parts, false, request); + return CompletableFuture.failedFuture(ex); + } + } else if (retries.isEmpty()) { + return null; + } else if (!lastRetries.isEmpty() && retries.containsAll(lastRetries)) { + // too bad: we seem to be in a retry cycle. Bail out. + LOG.log(Level.WARNING, "Project {0} is reloading repetadely. The following provider(s) reload in a loop: {1}", new Object[]{project.getProjectDirectory(), lastRetries}); + // FIXME: create a ProjectState from the data available so far. + ProjectOperationException ex = new ProjectOperationException(project, ProjectOperationException.State.ERROR, Bundle.ERR_ReloadingLoop(ProjectUtils.getInformation(project).getDisplayName(), lastRetries)); + registry.createState(originalState, project, variant, parts, false, request); + return CompletableFuture.failedFuture(ex); + } if (LOG.isLoggable(Level.FINE)) { - LOG.log(Level.FINE, "{0} load round completed.", toString()); + String s = Stream.concat(fileInconsistencies.stream(),retries.stream()). + map(c -> c.impl.getClass().getName()).collect(Collectors.joining(", ")); + LOG.log(Level.FINE, "{0} reloads again because of {1}", new Object[]{toString(), s}); + } + this.lastRetries = retries; + this.lastInconsistent = fileInconsistencies; + initRound(); + // unspecific reload: make all providers to reload their data. + this.forcedRound = forceReload; + reloadRound++; + return processOne(); + } + + // the same check as done in StateDataListener + private void checkFileTimestamps(Collection impls) { + Map> wf = new HashMap<>(); + for (LoadContextImpl ctx : this.loadData) { + ProjectStateData d = ctx.loadedData; + if (d == null) { + continue; + } + Collection files = d.getFiles(); + files.forEach(f -> { + wf.computeIfAbsent(f, x -> new ArrayList<>()).add(d); + }); } - if (!retries.isEmpty()) { - // let's allow the reload if at least of the last reloaders was 'satisfied' this time. - if (!lastRetries.isEmpty() && retries.containsAll(lastRetries)) { - // too bad: we seem to be in a retry cycle. Bail out. - LOG.log(Level.WARNING, "Project {0} is reloading repetadely. The following provider(s) reload in a loop: {1}", new Object[]{project.getProjectDirectory(), lastRetries}); - // FIXME: create a ProjectState from the data available so far. - ProjectOperationException ex = new ProjectOperationException(project, ProjectOperationException.State.ERROR, Bundle.ERR_ReloadingLoop(ProjectUtils.getInformation(project).getDisplayName(), lastRetries)); - error = ex; - } else { - if (LOG.isLoggable(Level.FINE)) { - String s = retries.stream().map(c -> c.impl.getClass().getName()).collect(Collectors.joining(", ")); + for (ProjectStateData d : checkFileTimestamps(this, wf)) { + this.loadData.stream().filter(x -> x.loadedData == d).forEach(i -> impls.add(i)); + } + } + + public static Collection checkFileTimestamps(Object s, Map> wf) { + Collection inconsistent = new ArrayList<>(); + for (Map.Entry> e : wf.entrySet()) { + FileObject f = e.getKey(); + f.refresh(); + long t = f.lastModified().getTime(); + boolean modified = f.getLookup().lookup(SaveCookie.class) != null; + for (ProjectStateData d : e.getValue()) { + if (modified || d.getTimestamp() < t) { if (LOG.isLoggable(Level.FINE)) { - LOG.log(Level.FINE, "{0} reloads again because of {1}", new Object[]{toString(), s}); + LOG.log(Level.FINER, "CHECK {0}: StateData not consistent: {1}. Modified={2}, file={3}, state={4}", + new Object[] { s, d.toString(), modified, t, d.getTimestamp() }); } + d.fireChanged(false, true); + inconsistent.add(d); } - this.lastRetries = retries; - initRound(); - // unspecific reload: make all providers to reload their data. - this.forcedRound = forceReload; - reloadRound++; - return processOne(); } } + return inconsistent; + } + + @NbBundle.Messages(value = {"# {0} - project name", "# {1} - list of providers in loop", "ERR_ReloadingLoop=Project is reloading repeatedly. See the log for more details.", "# {0} - project name", "ERR_ProjectModifiedWhileLoading=Project {0} has been modified while reloading", "# {0} - project name", "ERR_ProjectQualityLow=Project {0} could not be loaded."}) + private synchronized CompletableFuture finishLoadingRound() { + markInconsistentParts(); + if (LOG.isLoggable(Level.FINE)) { + LOG.log(Level.FINE, "{0} load round completed.", toString()); + } + Collection variant = ProjectReloadInternal.variantKey(project, parts, request.getContext()); + if (this.variantKey != null && !this.variantKey.equals(variant)) { + LOG.log(Level.WARNING, "Variant key formed for {0} differs from cache key: {1}/{2}", new Object[]{ + request, variant, variantKey + }); + } + + CompletableFuture f = maybeMakeRetry(variant); + if (f != null) { + return f; + } + Throwable error = null; ProjectReload.ProjectState result; + boolean finished = false; try { - synchronized (this) { - Collection variant = ProjectReloadInternal.variantKey(project, parts, request.getContext()); - if (this.variantKey != null && !this.variantKey.equals(variant)) { - LOG.log(Level.WARNING, "Variant key formed for {0} differs from cache key: {1}/{2}", new Object[]{ - request, variant, variantKey - }); - } - // state will be created even in the failed case, ensuring that the last-created ProjectStateData will - // become cached and the old ones will be marked as invalid. - result = registry.createState(originalState, project, variant, parts, true, request).second(); + // state will be created even in the failed case, ensuring that the last-created ProjectStateData will + // become cached and the old ones will be marked as invalid. Do not accept inconsistent state, yet. + result = registry.createState(originalState, project, variant, parts, request.isConsistent(), request).second(); + + if (LOG.isLoggable(Level.FINE)) { + LOG.log(Level.FINE, "{0} load round completed.", toString()); } + // this will error if there are repeated reloads. + f = maybeMakeRetry(variant); + if (f != null) { + return f; + } + finished = true; // post-check the result's quality and consistency to match the request - if ((!result.isConsistent()) && request.isConsistent()) { - if (LOG.isLoggable(Level.FINE)) { - LOG.log(Level.INFO, "{0} loaded as inconsistent, failing operation", toString()); - } - ProjectOperationException ex = new ProjectOperationException(project, ProjectOperationException.State.OUT_OF_SYNC, Bundle.ERR_ProjectModifiedWhileLoading(ProjectUtils.getInformation(project).getDisplayName())); - error = ex; - } else if (result.getQuality().isWorseThan(request.getMinQuality())) { + if (result.getQuality().isWorseThan(request.getMinQuality())) { ProjectOperationException ex = null; - Throwable first = null; for (LoadContextImpl d : loadData) { if (d.reloadError != null) { - Throwable t = d.reloadError.getCause(); + Throwable t = d.reloadError; if (t instanceof ProjectOperationException) { ex = (ProjectOperationException) t; break; - } else if (first == null) { - first = t; } } } if (ex == null) { - ex = new ProjectOperationException(project, ProjectOperationException.State.BROKEN, Bundle.ERR_ProjectQualityLow(ProjectUtils.getInformation(project).getDisplayName()), first); - } - for (LoadContextImpl d : loadData) { - Throwable e = d.reloadError; - if (e != null) { - Throwable c = e.getCause(); - if (ex != c && ex.getCause() != c && e != c) { - ex.addSuppressed(c); - } - } + ex = new ProjectOperationException(project, ProjectOperationException.State.ERROR, Bundle.ERR_ProjectQualityLow(ProjectUtils.getInformation(project).getDisplayName())); } error = ex; if (LOG.isLoggable(Level.FINE)) { LOG.log(Level.FINE, "{0} loaded as low quality ({1}/{2}), failing operation", new Object[]{toString(), result.getQuality(), request.getMinQuality()}); - LOG.log(Level.FINE, "Error is: {0}", error); } + } else if ((!result.isConsistent()) && request.isConsistent()) { + if (LOG.isLoggable(Level.FINE)) { + LOG.log(Level.FINE, "{0} loaded as inconsistent, failing operation", toString()); + } + ProjectOperationException ex = new ProjectOperationException(project, ProjectOperationException.State.OUT_OF_SYNC, Bundle.ERR_ProjectModifiedWhileLoading(ProjectUtils.getInformation(project).getDisplayName())); + error = ex; } if (error != null) { + LOG.log(Level.FINE, "{0}: Resulted in errors: {1}", new Object[] { this, error.getMessage() }); + LOG.log(Level.FINE, "Stacktrace: ", error); + boolean first = true; + for (LoadContextImpl d : loadData) { + Throwable e = d.reloadError; + if (e != null && error != e) { + if (LOG.isLoggable(Level.FINE)) { + LOG.log(Level.FINE, "Additional error:", e); + } + if (first && error.getCause() == null || error.getCause() != error) { + error.initCause(e); + } else { + error.addSuppressed(e); + } + first = false; + } + } return CompletableFuture.failedFuture(error); } else { return CompletableFuture.completedFuture(result); } } finally { - // go through all states acquired during the reload, and check which were registered for ProjectStates. - // The ones which are registered will be cleaned up when their identity object is GCed. - // But those that are NOT registered as identities need to be cleaned now. - for (Iterator, ProjectReloadImplementation.ProjectStateData>> it = reportedStates.iterator(); it.hasNext();) { - Pair, ProjectReloadImplementation.ProjectStateData> pair = it.next(); - if (parts.get(pair.first()) != pair.second()) { - if (LOG.isLoggable(Level.FINE)) { - LOG.log(Level.FINE, "{0}: Invalidating state {1}, final state has different instance", new Object[]{toString(), pair.second().toString()}); + if (finished) { + // clean up context as it is potentially held by SPIs. + loadData.forEach(ctx -> { + ctx.reinit(ProjectReloadInternal.EMPTY_PARTS); + ReloadSpiAccessor.get().clear(ctx.clientContext); + }); + // go through all states acquired during the reload, and check which were registered for ProjectStates. + // The ones which are registered will be cleaned up when their identity object is GCed. + // But those that are NOT registered as identities need to be cleaned now. + for (Iterator, ProjectReloadImplementation.ProjectStateData>> it = reportedStates.iterator(); it.hasNext();) { + Pair, ProjectReloadImplementation.ProjectStateData> pair = it.next(); + if (parts.get(pair.first()) != pair.second()) { + if (LOG.isLoggable(Level.FINE)) { + LOG.log(Level.FINE, "{0}: Invalidating state {1}, final state has different instance", new Object[]{toString(), pair.second().toString()}); + } + pair.second().fireChanged(true, false); } - pair.second().fireChanged(true, false); - } - if (registry.hasIdentity(pair.second())) { - it.remove(); - } - } - // run a delayed cleanup action for each of unknown states. - if (!reportedStates.isEmpty()) { - if (LOG.isLoggable(Level.FINE)) { - if (LOG.isLoggable(Level.FINE)) { - LOG.log(Level.FINE, "{0}: Scheduling cleanup of dangling states", new Object[]{toString(), - reportedStates.stream().map(p -> p.second().toString()).collect(Collectors.joining(", ")) - }); + if (registry.hasIdentity(pair.second())) { + it.remove(); } } - registry.runProjectAction(project, () -> { - reportedStates.forEach(pair -> { + // run a delayed cleanup action for each of unknown states. + if (!reportedStates.isEmpty()) { + if (LOG.isLoggable(Level.FINE)) { if (LOG.isLoggable(Level.FINE)) { - LOG.log(Level.FINE, "{0}: Releasing dangling states", toString()); - pair.first().projectDataReleased(pair.second()); - ReloadSpiAccessor.get().release(pair.second()); + LOG.log(Level.FINE, "{0}: Scheduling cleanup of dangling states", new Object[]{toString(), + reportedStates.stream().map(p -> p.second().toString()).collect(Collectors.joining(", ")) + }); } + } + registry.runProjectAction(project, () -> { + reportedStates.forEach(pair -> { + if (LOG.isLoggable(Level.FINE)) { + LOG.log(Level.FINE, "{0}: Releasing dangling states", toString()); + pair.first().projectDataReleased(pair.second()); + ReloadSpiAccessor.get().release(pair.second()); + } + }); }); - }); + } } } } @@ -589,6 +685,9 @@ private void loadStepDone1(ProjectStateData state, LoadContextImpl fd, Throwa if (t != null) { fd.reloadError = t; } + if (fd.clientContext != null) { + fd.contextData = fd.clientContext.getLoadContext(Object.class); + } fd.loadedData = state; fd.loadedOnce = true; synchronized (this) { @@ -646,6 +745,46 @@ public CompletableFuture start(RequestProcessor proc } private static final ProjectStateData CANCEL = ProjectStateData.builder(ProjectReload.Quality.NONE).build(); + + private ProjectStateData processGeneralError(LoadContextImpl fd, Throwable t) { + // synthesize a new ProjectStateData + ProjectReloadImplementation.ProjectStateBuilder b = ProjectReloadImplementation.ProjectStateData.builder(fd.loadedData == null ? ProjectReload.Quality.NONE : ProjectReload.Quality.BROKEN); + // ForwardDataChanges refires changes on the last impl-created state data on this fake instance. + class ForwardDataChanges implements ProjectStateListener { + + volatile ProjectReloadImplementation.ProjectStateData toFire; + + @Override + public void stateChanged(ChangeEvent e) { + // refire all file and validity changes. + ProjectReloadImplementation.ProjectStateData orig = (ProjectReloadImplementation.ProjectStateData) e.getSource(); + toFire.fireFileSetChanged(orig.getChangedFiles()); + toFire.fireChanged(!orig.isValid(), !orig.isConsistent()); + } + + @Override + public void fireDataInconsistent(ProjectStateData d, Class dataClass) { + toFire.fireDataInconsistent(dataClass); + } + } + ProjectReloadImplementation.ProjectStateData fakeD; + if (fd.loadedData != null) { + b.files(fd.loadedData.getFiles()); + b.state(fd.loadedData.isConsistent(), true); + b.timestamp(fd.loadedData.getTimestamp()); + ForwardDataChanges cl = new ForwardDataChanges(); + // this will keep the listener alive :-/ + b.attachLookup(Lookups.fixed(cl)); + fakeD = b.build(); + cl.toFire = fakeD; + ReloadSpiAccessor.get().addProjectStateListener(fd.loadedData, cl); + } else { + fakeD = b.build(); + } + fd.reportLoadError = !(t instanceof ProjectOperationException); + fd.reloadError = t; + return fakeD; + } /** * Processes one implementation participant. Sort of, because if a {@link ProjectReloadImplementation} @@ -684,7 +823,19 @@ synchronized CompletableFuture processOne() { if (q != null && q.isWorseThan(request.getMinQuality())) { break; } - if (!d.loadedData.isConsistent() || !d.loadedData.isValid()) { + if (!d.loadedData.isValid()) { + break; + } + long ts = d.loadedData.getTimestamp(); + Collection fos = d.loadedData.getFiles(); + for (FileObject f : fos) { + f.refresh(); + if (f.lastModified().getTime() > ts) { + d.loadedData.fireChanged(false, true); + break; + } + } + if (!d.loadedData.isConsistent() && request.isConsistent()) { break; } if (lastRetries.contains(d)) { @@ -714,7 +865,9 @@ synchronized CompletableFuture processOne() { if (cancelled != null) { return cancelLoadInProgress(); } - fd.clientContext = ReloadSpiAccessor.get().createLoadContext(fd); + if (fd.clientContext == null) { + fd.clientContext = ReloadSpiAccessor.get().createLoadContext(fd); + } newData = d.impl.reload(project, request, fd.clientContext); currentStage = newData; @@ -739,46 +892,14 @@ synchronized CompletableFuture processOne() { } if (t instanceof ProjectReloadImplementation.PartialLoadException) { ProjectReloadImplementation.PartialLoadException ple = (ProjectReloadImplementation.PartialLoadException) t; - fd.reloadError = ple; + Throwable cause = ple.getCause(); + if (cause == null) { + cause = ple; + } + fd.reloadError = cause; return ple.getPartialData(); } else { - // synthesize a new ProjectStateData - ProjectReloadImplementation.ProjectStateBuilder b = ProjectReloadImplementation.ProjectStateData.builder(fd.loadedData == null ? ProjectReload.Quality.NONE : ProjectReload.Quality.BROKEN); - - // ForwardDataChanges refires changes on the last impl-created state data on this fake instance. - class ForwardDataChanges implements ProjectStateListener { - - volatile ProjectReloadImplementation.ProjectStateData toFire; - - @Override - public void stateChanged(ChangeEvent e) { - // refire all file and validity changes. - ProjectReloadImplementation.ProjectStateData orig = (ProjectReloadImplementation.ProjectStateData) e.getSource(); - toFire.fireFileSetChanged(orig.getChangedFiles()); - toFire.fireChanged(!orig.isValid(), !orig.isConsistent()); - } - - @Override - public void fireDataInconsistent(ProjectStateData d, Class dataClass) { - toFire.fireDataInconsistent(dataClass); - } - } - ProjectReloadImplementation.ProjectStateData fakeD; - if (fd.loadedData != null) { - b.files(fd.loadedData.getFiles()); - b.state(fd.loadedData.isConsistent(), true); - b.timestamp(fd.loadedData.getTimestamp()); - ForwardDataChanges cl = new ForwardDataChanges(); - // this will keep the listener alive :-/ - b.attachLookup(Lookups.fixed(cl)); - fakeD = b.build(); - cl.toFire = fakeD; - ReloadSpiAccessor.get().addProjectStateListener(fd.loadedData, cl); - } else { - fakeD = b.build(); - } - fd.reloadError = t; - return fakeD; + return processGeneralError(fd, t); } }); @@ -797,6 +918,11 @@ public void fireDataInconsistent(ProjectStateData d, Class dataClass) { } ProjectReloadImplementation.PartialLoadException ple = (ProjectReloadImplementation.PartialLoadException) ex; return loadStepDone(ple.getPartialData(), fd, ple); + } catch (ThreadDeath td) { + throw td; + } catch (Throwable ex) { + ProjectStateData data = processGeneralError(fd, ex); + return loadStepDone(data, fd, ex); } } } diff --git a/ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/StateDataListener.java b/ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/StateDataListener.java index 9bb2f70d9a2c..5aec2c649cf5 100644 --- a/ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/StateDataListener.java +++ b/ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/StateDataListener.java @@ -93,6 +93,28 @@ void init() { for (ProjectReloadImplementation.ProjectStateData d : parts.values()) { ReloadSpiAccessor.get().addProjectStateListener(d, this); } + checkFileTimestamps(); + checkInconsistencies(); + } + + + public void checkFileTimestamps() { + ProjectState s = tracker.get(); + if (s == null) { + return; + } + Map> wf = watchedFiles; + Reloader.checkFileTimestamps(s, wf); + } + + void checkInconsistencies() { + ProjectState s = tracker.get(); + if (s == null) { + return; + } + for (ProjectStateData d : parts.values()) { + Reloader.markInconsistencies(d, null, parts, s); + } } @Override @@ -263,7 +285,7 @@ public void fileDeleted(FileEvent fe) { @Override public void fileChanged(FileEvent fe) { - reportFile(fe.getFile(), -1); + reportFile(fe.getFile(), fe.getFile().lastModified().getTime()); } @Override @@ -290,8 +312,8 @@ public void cookiesChanged(FileObject f, LookupEvent ev) { } ReloadApiAccessor.get().updateProjectState(state, true, false, ch, ed, null); } + watchedFiles.getOrDefault(f, Collections.emptyList()).forEach(d -> d.fireChanged(false, true)); } - watchedFiles.getOrDefault(f, Collections.emptyList()).forEach(d -> d.fireChanged(false, true)); } } diff --git a/ide/project.dependency/src/org/netbeans/modules/project/dependency/spi/ProjectReloadImplementation.java b/ide/project.dependency/src/org/netbeans/modules/project/dependency/spi/ProjectReloadImplementation.java index 84eb99b14a0d..ba541666b872 100644 --- a/ide/project.dependency/src/org/netbeans/modules/project/dependency/spi/ProjectReloadImplementation.java +++ b/ide/project.dependency/src/org/netbeans/modules/project/dependency/spi/ProjectReloadImplementation.java @@ -166,7 +166,7 @@ public final class ProjectStateData { this.timestamp = timestamp; this.projectData = projectData; this.error = error; - if (!valid && LOG.isLoggable(Level.FINER)) { + if (!valid && quality != Quality.NONE && LOG.isLoggable(Level.FINER)) { LOG.log(Level.FINER, "Created invalid state data {0}", toString()); LOG.log(Level.FINER, "Origin", new Throwable()); } @@ -334,17 +334,24 @@ public void fireChanged(boolean invalidate, boolean inconsistent) { LOG.log(Level.FINER, "{0} got state change: invalidate={1}, inconsistent={2}", new Object[] { toString(), invalidate, inconsistent }); LOG.log(Level.FINEST, "Origin:", new Throwable()); } - if (invalidate) { - this.valid = false; + boolean changed = false; + synchronized (this) { + if (invalidate && this.valid) { + this.valid = false; + changed = true; + } + if (inconsistent && this.consistent) { + changed = true; + this.consistent = false; + } } - if (inconsistent) { - this.consistent = false; + if (changed) { + fire(); } - fire(); } Set getInconsistencies() { - return inconsistencies; + return inconsistencies == null ? Collections.emptySet() : new HashSet<>(inconsistencies); } synchronized void addListener(ProjectStateListener l) { @@ -368,12 +375,14 @@ void clear() { } } - this.projectData = null; - this.changedFiles = null; - this.privateData = null; - this.lookup = null; - this.projectData = null; - this.valid = false; + synchronized (this) { + this.projectData = null; + this.changedFiles = null; + this.privateData = null; + this.lookup = null; + this.projectData = null; + this.valid = false; + } LOG.log(Level.FINER, "{0}: cleared.", toString()); this.listeners.clear(); } @@ -500,6 +509,10 @@ public ProjectStateBuilder privateData(Function, X> crea public ProjectStateData build() { ProjectStateData d = new ProjectStateData(files == null ? Collections.emptyList() : files, valid, q, time, lkp, error, data); + if (!consistent) { + // make inconsistent + d.fireChanged(false, true); + } if (!d.isValid() || !d.isConsistent()) { if (Reloader.LOG.isLoggable(Level.FINER)) { Reloader.LOG.log(Level.FINER, "Creating obsolete StateData: {0}", d.toString()); @@ -754,7 +767,8 @@ public T getLoadContext(Class clazz) { } public T ensureLoadContext(Class clazz, Supplier factory) { - if (loadContext == null) { + T lc = getLoadContext(clazz); + if (lc == null) { loadContext = factory.get(); } return (T)loadContext; diff --git a/ide/project.dependency/test/unit/src/org/netbeans/modules/project/dependency/reload/MockProjectReloadImplementation.java b/ide/project.dependency/test/unit/src/org/netbeans/modules/project/dependency/reload/MockProjectReloadImplementation.java index 73d52158e1f5..c04f9e768936 100644 --- a/ide/project.dependency/test/unit/src/org/netbeans/modules/project/dependency/reload/MockProjectReloadImplementation.java +++ b/ide/project.dependency/test/unit/src/org/netbeans/modules/project/dependency/reload/MockProjectReloadImplementation.java @@ -58,6 +58,7 @@ public void setProject(Project p) { public static class ProjectData { public String contents; + public volatile int counter; public ProjectData(String s) { this.contents = s; @@ -118,10 +119,13 @@ public void projectDataReleased(ProjectStateData data) { Reference sdRef = new WeakReference<>(null); + volatile ProjectData loadProjectData; + protected ProjectStateData doCreateStateData(Project project, ProjectReload.StateRequest request, LoadContext context) { ProjectStateBuilder b = ProjectStateData.builder( request.getMinQuality().isAtLeast(ProjectReload.Quality.LOADED) ? request.getMinQuality() : ProjectReload.Quality.LOADED); b.timestamp(Instant.now().toEpochMilli()); + loadProjectData = context.getLoadContext(ProjectData.class); ProjectStateData psd = createStateData(b, request). privateData( (d) -> { diff --git a/ide/project.dependency/test/unit/src/org/netbeans/modules/project/dependency/reload/ProjectReloadImplTest.java b/ide/project.dependency/test/unit/src/org/netbeans/modules/project/dependency/reload/ProjectReloadImplTest.java index 3001f9da1b58..63afc825eded 100644 --- a/ide/project.dependency/test/unit/src/org/netbeans/modules/project/dependency/reload/ProjectReloadImplTest.java +++ b/ide/project.dependency/test/unit/src/org/netbeans/modules/project/dependency/reload/ProjectReloadImplTest.java @@ -55,6 +55,7 @@ import org.netbeans.modules.project.dependency.ProjectOperationException; import org.netbeans.modules.project.dependency.ProjectReload; import org.netbeans.modules.project.dependency.ProjectReload.ProjectState; +import org.netbeans.modules.project.dependency.ProjectReload.Quality; import org.netbeans.modules.project.dependency.ProjectReload.StateRequest; import org.netbeans.modules.project.dependency.reload.MockProjectReloadImplementation.ProjectData; import org.netbeans.modules.project.dependency.reload.ProjectReloadImplTest.Mock1; @@ -80,21 +81,24 @@ public ProjectReloadImplTest(String name) { super(name); } - Logger logger; + Collection loggers = new ArrayList<>(); @Override protected void setUp() throws Exception { super.setUp(); clearWorkDir(); - logger = Logger.getLogger(ProjectReloadInternal.class.getName()); - logger.setLevel(Level.FINER); - - ConsoleHandler h = new ConsoleHandler(); - h.setLevel(Level.FINER); - if (!Arrays.asList(logger.getHandlers()).stream().anyMatch(x -> x instanceof ConsoleHandler)) { - logger.addHandler(h); - } + loggers.add(Logger.getLogger(ProjectReloadInternal.class.getName())); + loggers.add(Logger.getLogger(Reloader.class.getName())); + loggers.forEach(logger -> { + logger.setLevel(Level.FINER); + + ConsoleHandler h = new ConsoleHandler(); + h.setLevel(Level.FINER); + if (!Arrays.asList(logger.getHandlers()).stream().anyMatch(x -> x instanceof ConsoleHandler)) { + logger.addHandler(h); + } + }); } @Override @@ -1400,6 +1404,9 @@ class WaitMock extends MockProjectReloadImplementation { * Released when reload is reached, for test synchronization */ Semaphore reloadReached = new Semaphore(0); + Semaphore afterReloadReached = new Semaphore(0); + + volatile StateRequest loadRequest; /** * Called will be used as a index to this @@ -1416,6 +1423,7 @@ public WaitMock() { @Override public CompletableFuture> reload(Project project, StateRequest request, ProjectReloadImplementation.LoadContext context) { + this.loadRequest = request; synchronized (this) { int n = called.getAndIncrement(); if (attemptReached != null && attemptReached.length > n) { @@ -1424,6 +1432,7 @@ public CompletableFuture> reload(Project project, } reloadReached.release(); CompletableFuture> f = super.reload(project, request, context); + afterReloadReached.release(); return f.thenCombine(CompletableFuture.runAsync(() -> { try { latch.acquire(); @@ -1601,4 +1610,67 @@ public void stateChanged(ChangeEvent e) { // wait a litte, as these postponed actions are executed after the reload Future completes assertTrue(m1.dataReleaseLatch.tryAcquire(2, TimeUnit.SECONDS)); } + + + /** + * Checks that if a file is modified during reload, the API will not report + * a failure, but attempts to retry the load. + */ + public void testFileModifiedDuringLoad() throws Exception { + FileObject wd = FileUtil.toFileObject(getWorkDir()); + FileObject o = FileUtil.toFileObject(getDataDir()).getFileObject("reload/Simple1._test"); + FileObject f = FileUtil.copyFile(o, wd, wd.getName()); + + class WM extends WaitMock { + public WM() { + } + + @Override + protected ProjectStateData doCreateStateData(Project project, StateRequest request, LoadContext context) { + context.ensureLoadContext(ProjectData.class, () -> new ProjectData("")).counter++; + return super.doCreateStateData(project, request, context); + } + + protected ProjectStateBuilder createStateData(ProjectReloadImplementation.ProjectStateBuilder b, ProjectReload.StateRequest request) { + ProjectStateBuilder b2 = super.createStateData(b, request); + return b2; + } + } + + WM m1 = new WM(); + + TestProjectFactory.addToProject(f, (p) -> { + m1.project = p; + return m1; + }); + + Project p = ProjectManager.getDefault().findProject(f); + + CompletableFuture future = ProjectReload.withProjectState(p, StateRequest.refresh()); + + m1.afterReloadReached.acquire(); + + FileObject pf = f.getFileObject("project.txt"); + Thread.sleep(2000); + Files.setLastModifiedTime(FileUtil.toFile(pf).toPath(), FileTime.from(Instant.now())); + // continue loading, this release will serve for the INITIAL non-NONE load state + assertSame(Quality.NONE, m1.loadRequest.getMinQuality()); + assertEquals(1, m1.loadProjectData.counter); + m1.latch.release(); + + m1.afterReloadReached.acquire(); + Thread.sleep(2000); + Files.setLastModifiedTime(FileUtil.toFile(pf).toPath(), FileTime.from(Instant.now())); + assertEquals(1, m1.loadProjectData.counter); + m1.latch.release(); + + m1.afterReloadReached.acquire(); + assertEquals("The load goes through ReloadImplementation 2nd time", 2, m1.loadProjectData.counter); + m1.latch.release(); + + ProjectState ps = future.get(); + + assertTrue("The result is the most recent", ps.isValid()); + assertTrue("Consistent after reload", ps.isConsistent()); + } } diff --git a/java/gradle.dependencies/src/org/netbeans/modules/gradle/reload/GradleReloadImplementation.java b/java/gradle.dependencies/src/org/netbeans/modules/gradle/reload/GradleReloadImplementation.java index 3708508de557..1422955edb8a 100644 --- a/java/gradle.dependencies/src/org/netbeans/modules/gradle/reload/GradleReloadImplementation.java +++ b/java/gradle.dependencies/src/org/netbeans/modules/gradle/reload/GradleReloadImplementation.java @@ -74,6 +74,11 @@ public GradleReloadImplementation(Project project) { gp.addPropertyChangeListener(WeakListeners.propertyChange(reloadL, gp)); } + // tests only + public ProjectStateData getLastCachedData() { + return last.get(); + } + private static void includeFile(File f, Collection into) { if (f == null) { return; @@ -151,15 +156,9 @@ public ProjectStateData getProjectData() { ProjectStateData sd = ProjectStateData.builder(q). files(files). attachLookup(NbGradleProject.get(project).curretLookup()). + timestamp(time). build(); - /* - ProjectStateControl track = new ProjectStateControl( - "gradle", null, files, pq == NbGradleProject.Quality.FALLBACK || pq.betterThan(NbGradleProject.Quality.EVALUATED), - q, time, NbGradleProject.get(project).projectDataLookup() - ); - */ synchronized (this) { -// states.add(new WeakReference<>(track)); last = new WeakReference<>(sd); } return sd; diff --git a/java/gradle.dependencies/test/unit/src/org/netbeans/modules/gradle/reload/GradleReloadImplementationTest.java b/java/gradle.dependencies/test/unit/src/org/netbeans/modules/gradle/reload/GradleReloadImplementationTest.java index 141ce198d157..ae5c6dc57e14 100644 --- a/java/gradle.dependencies/test/unit/src/org/netbeans/modules/gradle/reload/GradleReloadImplementationTest.java +++ b/java/gradle.dependencies/test/unit/src/org/netbeans/modules/gradle/reload/GradleReloadImplementationTest.java @@ -25,6 +25,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.attribute.FileTime; +import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -32,14 +33,19 @@ import java.util.List; import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Semaphore; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; import javax.swing.text.Document; import static junit.framework.TestCase.assertEquals; +import static junit.framework.TestCase.assertFalse; import static junit.framework.TestCase.assertNotNull; +import static junit.framework.TestCase.assertNotSame; +import static junit.framework.TestCase.assertTrue; import static junit.framework.TestCase.fail; import org.gradle.tooling.internal.consumer.ConnectorServices; import org.netbeans.api.project.Project; @@ -54,6 +60,9 @@ import org.netbeans.modules.project.dependency.ProjectReload; import org.netbeans.modules.project.dependency.ProjectReload.ProjectState; import org.netbeans.modules.project.dependency.ProjectReload.StateRequest; +import org.netbeans.modules.project.dependency.spi.ProjectReloadImplementation; +import org.netbeans.modules.project.dependency.spi.ProjectReloadImplementation.LoadContext; +import org.netbeans.spi.project.ProjectServiceProvider; import org.openide.DialogDescriptor; import org.openide.DialogDisplayer; import org.openide.NotifyDescriptor; @@ -61,6 +70,7 @@ import org.openide.filesystems.FileObject; import org.openide.filesystems.FileUtil; import org.openide.modules.DummyInstalledFileLocator; +import org.openide.util.Exceptions; import org.openide.util.test.MockLookup; import org.openide.windows.IOProvider; @@ -596,4 +606,191 @@ public void testBetterQualityLoadsInconsistent() throws Exception { NbGradleProject gp = NbGradleProject.get(project); assertTrue(gp.getQuality().atLeast(NbGradleProject.Quality.EVALUATED)); } + + class Blocker implements Runnable { + CountDownLatch unblock = new CountDownLatch(1); + + @Override + public void run() { + try { + unblock.await(); + } catch (InterruptedException ex) { + Exceptions.printStackTrace(ex); + } + } + } + + /** + * Checks that if project data becomes obsolete during load (i.e. file modified), + * the reload API retries and produces a final state that is consistent. + */ + public void testProjectObsoletesDuringLoad() throws Exception { + setupSimpleProject(); + + FileObject script = prjDir.getFileObject("build.gradle"); + Path scriptPath = FileUtil.toFile(script).toPath(); + + ProjectState ps = ProjectReload.getProjectState(project, true); + + GradleReloadImplementation mri = (GradleReloadImplementation)project.getLookup().lookupAll(ProjectReloadImplementation.class).stream(). + filter(i -> i instanceof GradleReloadImplementation).findFirst().get(); + + MonitoringReloadImplementation.INSTANCE.enabled = true; + + Thread.sleep(2000); + Files.setLastModifiedTime(scriptPath, FileTime.from(Instant.now())); + script.refresh(); + + MonitoringReloadImplementation.INSTANCE.counter.set(0); + MonitoringReloadImplementation.INSTANCE.reloadReached1.drainPermits(); + MonitoringReloadImplementation.INSTANCE.proceed.drainPermits(); + + CompletableFuture newState = ProjectReload.withProjectState(project, StateRequest.refresh()); + + MonitoringReloadImplementation.INSTANCE.reloadReached1.acquire(); + + assertFalse(newState.isDone()); + int cnt = MonitoringReloadImplementation.INSTANCE.counter.get(); + ProjectReloadImplementation.ProjectStateData firstRound = mri.getLastCachedData(); + + assertTrue(firstRound.isConsistent()); + + Thread.sleep(2000); + + Instant firstMod = Instant.now(); + Files.setLastModifiedTime(scriptPath, FileTime.from(firstMod)); + // deliberately not calling refresh(). + MonitoringReloadImplementation.INSTANCE.proceed.release(1); + + // 2nd reload + MonitoringReloadImplementation.INSTANCE.reloadReached1.acquire(); + ProjectReloadImplementation.ProjectStateData secondRound = mri.getLastCachedData(); + // new state is OK, but the previous should be obsoleted. + assertNotSame(secondRound, firstRound); + assertTrue(secondRound.isConsistent()); + assertFalse(firstRound.isConsistent()); + + MonitoringReloadImplementation.INSTANCE.proceed.release(100); + ProjectState ps2 = newState.get(); + + assertFalse(ps.isValid()); + assertFalse(ps.isConsistent()); + + assertTrue(ps2.isValid()); + assertTrue(ps2.isConsistent()); + assertTrue(ps2.getQuality().isAtLeast(ProjectReload.Quality.LOADED)); + } + + /** + * Checks that if a project is repeatedly changed during the load, + * the operation fails with OUT_OF_SYNC exception. + */ + public void testRepeatedChangesErrorOut() throws Exception { + setupSimpleProject(); + + FileObject script = prjDir.getFileObject("build.gradle"); + Path scriptPath = FileUtil.toFile(script).toPath(); + + ProjectState ps = ProjectReload.getProjectState(project, true); + + GradleReloadImplementation gri = (GradleReloadImplementation)project.getLookup().lookupAll(ProjectReloadImplementation.class).stream(). + filter(i -> i instanceof GradleReloadImplementation).findFirst().get(); + + MonitoringReloadImplementation.INSTANCE.enabled = true; + + Thread.sleep(2000); + Instant baseMod = Instant.now(); + Files.setLastModifiedTime(scriptPath, FileTime.from(baseMod)); + script.refresh(); + + MonitoringReloadImplementation.INSTANCE.counter.set(0); + MonitoringReloadImplementation.INSTANCE.reloadReached1.drainPermits(); + MonitoringReloadImplementation.INSTANCE.proceed.drainPermits(); + + CompletableFuture newState = ProjectReload.withProjectState(project, StateRequest.refresh()); + + MonitoringReloadImplementation.INSTANCE.reloadReached1.acquire(); + + assertFalse(newState.isDone()); + int cnt = MonitoringReloadImplementation.INSTANCE.counter.get(); + ProjectReloadImplementation.ProjectStateData firstRound = gri.getLastCachedData(); + + assertTrue(firstRound.isConsistent()); + + Thread.sleep(2000); + + Instant firstMod = Instant.now(); + Files.setLastModifiedTime(scriptPath, FileTime.from(firstMod)); + // deliberately not calling refresh(). + MonitoringReloadImplementation.INSTANCE.proceed.release(1); + + // 2nd reload + MonitoringReloadImplementation.INSTANCE.reloadReached1.acquire(); + ProjectReloadImplementation.ProjectStateData secondRound = gri.getLastCachedData(); + // new state is OK, but the previous should be obsoleted. + assertNotSame(secondRound, firstRound); + assertTrue(secondRound.isConsistent()); + assertFalse(firstRound.isConsistent()); + + Thread.sleep(2000); + Instant secondMod = Instant.now(); + Files.setLastModifiedTime(scriptPath, FileTime.from(secondMod)); + MonitoringReloadImplementation.INSTANCE.proceed.release(100); + + try { + ProjectState ps2 = newState.get(); + fail("Should report an exception"); + } catch (ExecutionException ex) { + assertTrue(ex.getCause() instanceof ProjectOperationException); + ProjectOperationException pex = (ProjectOperationException)ex.getCause(); + assertSame(ProjectOperationException.State.OUT_OF_SYNC, pex.getState()); + } + + ProjectState ps2 = ProjectReload.getProjectState(project); + // may be valid or not, since Maven internally reloads the project independently, and + // will invalidate the state after load. + assertFalse(ps2.isConsistent()); + + assertTrue(ps2.getTimestamp() > baseMod.toEpochMilli()); + // will be at least the 1st modification + assertTrue(ps2.getTimestamp() >= firstMod.toEpochMilli()); + // and NOT the 2nd modification (modified twice -> error) + assertTrue(ps2.getTimestamp() < secondMod.toEpochMilli()); + + ProjectState ps3 = ProjectReload.withProjectState(project, StateRequest.refresh()).get(); + assertTrue(ps3.isValid()); + assertTrue(ps3.isConsistent()); + assertTrue(ps3.getTimestamp() >= secondMod.toEpochMilli()); + } + + @ProjectServiceProvider(service = ProjectReloadImplementation.class, projectType = NbGradleProject.GRADLE_PROJECT_TYPE) + public static class MonitoringReloadImplementation implements ProjectReloadImplementation { + static MonitoringReloadImplementation INSTANCE; + final Semaphore proceed = new Semaphore(0); + final Semaphore reloadReached1 = new Semaphore(0); + final Semaphore reloadReached2 = new Semaphore(0); + volatile boolean enabled; + AtomicInteger counter = new AtomicInteger(); + + public MonitoringReloadImplementation() { + INSTANCE = this; + } + + + @Override + public CompletableFuture reload(Project project, StateRequest request, LoadContext context) { + if (!enabled) { + return null; + } + reloadReached1.release(); + try { + proceed.acquire(); + counter.incrementAndGet(); + } catch (InterruptedException ex) { + Exceptions.printStackTrace(ex); + } + reloadReached2.release(); + return null; + } + } } diff --git a/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java b/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java index 1ba67b9cab3b..5357b99fcae9 100644 --- a/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java +++ b/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java @@ -1792,7 +1792,9 @@ public void didSave(DidSaveTextDocumentParams savedParams) { org.openide.util.Task t = (org.openide.util.Task)reload.invoke(cake); // wait for a limited time, this could be enough for the reload to complete, blocking LSP queue. We do not want to block LSP queue indefinitely: // in case of an error, the server could become unresponsive. - t.waitFinished(300); + if (!t.waitFinished(300)) { + LOG.log(Level.WARNING, "{0}: document reload did not finish in 300ms", file); + } } catch (ReflectiveOperationException | InterruptedException | SecurityException ex) { // nop } diff --git a/java/maven/src/org/netbeans/modules/maven/queries/MavenPrimingReloadImplementation.java b/java/maven/src/org/netbeans/modules/maven/queries/MavenPrimingReloadImplementation.java index c2fa886ce3dd..396624a70fc8 100644 --- a/java/maven/src/org/netbeans/modules/maven/queries/MavenPrimingReloadImplementation.java +++ b/java/maven/src/org/netbeans/modules/maven/queries/MavenPrimingReloadImplementation.java @@ -159,7 +159,6 @@ public CompletableFuture reload(Project project, ProjectReload.StateRequest requ synchronized (this) { placeholderArtifactNames.clear(); } - context.saveLoadContext(this); return CompletableFuture.completedFuture(null); } diff --git a/java/maven/src/org/netbeans/modules/maven/queries/MavenReloadImplementation.java b/java/maven/src/org/netbeans/modules/maven/queries/MavenReloadImplementation.java index b5da1fffe029..246cfa189d71 100644 --- a/java/maven/src/org/netbeans/modules/maven/queries/MavenReloadImplementation.java +++ b/java/maven/src/org/netbeans/modules/maven/queries/MavenReloadImplementation.java @@ -77,6 +77,18 @@ public MavenReloadImplementation(Project project) { ); } + @Override + public void projectDataReleased(ProjectStateData data) { + if (lastData.get() == data) { + lastData.clear(); + } + } + + // test only + ProjectStateData getLastCachedData() { + return lastData.get(); + } + @Override public void propertyChange(PropertyChangeEvent evt) { if (evt.getPropertyName() == null) { @@ -260,7 +272,7 @@ private void loadMavenProject2(MavenProject p, StateRequest request, CF future) } loadMavenProject3(future); } - + private void loadMavenProject3(CF future) { NbMavenProjectImpl nbImpl = project.getLookup().lookup(NbMavenProjectImpl.class); MavenProject current = nbImpl.getOriginalMavenProjectOrNull(); diff --git a/java/maven/test/unit/src/org/netbeans/modules/maven/queries/MavenReloadImplementationTest.java b/java/maven/test/unit/src/org/netbeans/modules/maven/queries/MavenReloadImplementationTest.java index 72323473fda1..13886a238c5e 100644 --- a/java/maven/test/unit/src/org/netbeans/modules/maven/queries/MavenReloadImplementationTest.java +++ b/java/maven/test/unit/src/org/netbeans/modules/maven/queries/MavenReloadImplementationTest.java @@ -24,14 +24,20 @@ import java.nio.file.Paths; import java.nio.file.attribute.FileTime; import java.time.Instant; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Comparator; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import java.util.logging.ConsoleHandler; +import java.util.logging.Level; +import java.util.logging.Logger; import javax.swing.text.Document; import org.apache.maven.project.MavenProject; import org.netbeans.api.project.FileOwnerQuery; @@ -49,12 +55,17 @@ import org.netbeans.modules.project.dependency.ProjectReload.Quality; import org.netbeans.modules.project.dependency.ProjectReload.StateRequest; import org.netbeans.modules.project.dependency.reload.ProjectReloadInternal; +import org.netbeans.modules.project.dependency.reload.Reloader; +import org.netbeans.modules.project.dependency.spi.ProjectReloadImplementation; +import org.netbeans.modules.project.dependency.spi.ProjectReloadImplementation.ProjectStateData; import org.netbeans.spi.project.ActionProgress; import org.netbeans.spi.project.ActionProvider; +import org.netbeans.spi.project.ProjectServiceProvider; import org.openide.cookies.EditorCookie; import org.openide.filesystems.FileObject; import org.openide.filesystems.FileUtil; import org.openide.modules.DummyInstalledFileLocator; +import org.openide.util.Exceptions; import org.openide.util.Lookup; import org.openide.util.lookup.Lookups; import org.openide.windows.IOProvider; @@ -69,6 +80,8 @@ public class MavenReloadImplementationTest extends NbTestCase { private FileObject repoFO; private FileObject dataFO; + Collection loggers = new ArrayList<>(); + public MavenReloadImplementationTest(String name) { super(name); } @@ -101,11 +114,26 @@ private static File getTestNBDestDir() { File destDirF = getTestNBDestDir(); DummyInstalledFileLocator.registerDestDir(destDirF); + /* System.err.println("*** Running: " + getName()); + + loggers.add(Logger.getLogger(ProjectReloadInternal.class.getName())); + loggers.add(Logger.getLogger(Reloader.class.getName())); + loggers.forEach(logger -> { + logger.setLevel(Level.FINER); + + ConsoleHandler h = new ConsoleHandler(); + h.setLevel(Level.FINER); + if (!Arrays.asList(logger.getHandlers()).stream().anyMatch(x -> x instanceof ConsoleHandler)) { + logger.addHandler(h); + } + }); + */ } @Override protected void tearDown() throws Exception { + MonitoringReloadImplementation.INSTANCE.enabled = false; OpenProjects.getDefault().close(OpenProjects.getDefault().getOpenProjects()); ProjectReloadInternal.getInstance().assertNoOperations(); super.tearDown(); @@ -598,4 +626,226 @@ public void testMavenLoadsNewProjectAfterInternalReload() throws Exception { assertEquals(time, s.getTimestamp()); }).get(); } + + class Blocker implements Runnable { + CountDownLatch unblock = new CountDownLatch(1); + + @Override + public void run() { + try { + unblock.await(); + } catch (InterruptedException ex) { + Exceptions.printStackTrace(ex); + } + } + } + /** + * Simulates that a Maven project load is scheduled after a file change, but + * Reload API is invoked BEFORE that load completes. The cached Maven project + * exists, but with an old timestamp. getProjectState() should return + * inconsistent even in this case. + * + * @throws Exception + */ + public void testInitialLoadObsoleteProject() throws Exception { + setupMicronautProject(); + primeProject(); + + FileObject pom = oci.getProjectDirectory().getFileObject("pom.xml"); + Path pomPath = FileUtil.toFile(pom).toPath(); + + Blocker blocker = new Blocker(); + + NbMavenProjectImpl.RELOAD_RP.post(blocker); + try { + Files.setLastModifiedTime(pomPath, FileTime.from(Instant.now())); + pom.refresh(); + + ProjectState ps = ProjectReload.getProjectState(oci, true); + + assertFalse(ps.isConsistent()); + MavenReloadImplementation mri = (MavenReloadImplementation)oci.getLookup().lookupAll(ProjectReloadImplementation.class).stream(). + filter(i -> i instanceof MavenReloadImplementation).findFirst().get(); + ProjectStateData psd = mri.getLastCachedData(); + assertFalse(psd.isConsistent()); + } finally { + blocker.unblock.countDown(); + } + } + + /** + * Checks that if a project becomes inconsistent during the state load, the state will internally + * reload before returning the result. + * + * @throws Exception + */ + public void testProjectObsoletesDuringLoad() throws Exception { + setupMicronautProject(); + primeProject(); + + FileObject pom = oci.getProjectDirectory().getFileObject("pom.xml"); + Path pomPath = FileUtil.toFile(pom).toPath(); + + ProjectState ps = ProjectReload.getProjectState(oci, true); + + MavenReloadImplementation mri = (MavenReloadImplementation)oci.getLookup().lookupAll(ProjectReloadImplementation.class).stream(). + filter(i -> i instanceof MavenReloadImplementation).findFirst().get(); + + MonitoringReloadImplementation.INSTANCE.enabled = true; + + Thread.sleep(2000); + Files.setLastModifiedTime(pomPath, FileTime.from(Instant.now())); + pom.refresh(); + + MonitoringReloadImplementation.INSTANCE.counter.set(0); + MonitoringReloadImplementation.INSTANCE.reloadReached1.drainPermits(); + MonitoringReloadImplementation.INSTANCE.proceed.drainPermits(); + + CompletableFuture newState = ProjectReload.withProjectState(oci, StateRequest.refresh()); + + MonitoringReloadImplementation.INSTANCE.reloadReached1.acquire(); + assertFalse(newState.isDone()); + int cnt = MonitoringReloadImplementation.INSTANCE.counter.get(); + ProjectStateData firstRound = mri.getLastCachedData(); + + Thread.sleep(2000); + Files.setLastModifiedTime(pomPath, FileTime.from(Instant.now())); + // deliberately not calling refresh(). + + MonitoringReloadImplementation.INSTANCE.proceed.release(100); + + ProjectState ps2 = newState.get(); + + assertTrue(ps2.isValid()); + assertTrue(ps2.isConsistent()); + + ProjectStateData secondRound = mri.getLastCachedData(); + assertNotSame(firstRound, secondRound); + + // one reload happened. + assertEquals(2, MonitoringReloadImplementation.INSTANCE.counter.get()); + } + + /** + * Checks that repeated file changes will not loop the reloads, but fail. + * + * @throws Exception + */ + public void testRepeatedChangesErrorOut() throws Exception { + setupMicronautProject(); + primeProject(); + + FileObject pom = oci.getProjectDirectory().getFileObject("pom.xml"); + Path pomPath = FileUtil.toFile(pom).toPath(); + + ProjectState ps = ProjectReload.getProjectState(oci, true); + + MavenReloadImplementation mri = (MavenReloadImplementation)oci.getLookup().lookupAll(ProjectReloadImplementation.class).stream(). + filter(i -> i instanceof MavenReloadImplementation).findFirst().get(); + + MonitoringReloadImplementation.INSTANCE.enabled = true; + + Blocker blocker = new Blocker(); + + Instant baseMod = Instant.now(); + Files.setLastModifiedTime(pomPath, FileTime.from(baseMod)); + pom.refresh(); + + MonitoringReloadImplementation.INSTANCE.counter.set(0); + MonitoringReloadImplementation.INSTANCE.reloadReached1.drainPermits(); + MonitoringReloadImplementation.INSTANCE.proceed.drainPermits(); + + CompletableFuture newState = ProjectReload.withProjectState(oci, StateRequest.refresh()); + + // 1st reload + MonitoringReloadImplementation.INSTANCE.reloadReached1.acquire(); + + assertFalse(newState.isDone()); + int cnt = MonitoringReloadImplementation.INSTANCE.counter.get(); + ProjectStateData firstRound = mri.getLastCachedData(); + assertTrue(firstRound.isConsistent()); + + Thread.sleep(2000); + + // prevent Maven from reloading the project + NbMavenProjectImpl.RELOAD_RP.post(blocker); + + Instant firstMod = Instant.now(); + Files.setLastModifiedTime(pomPath, FileTime.from(firstMod)); + // deliberately not calling refresh(). + + MonitoringReloadImplementation.INSTANCE.proceed.release(1); + + blocker.unblock.countDown(); + + // 2nd reload + MonitoringReloadImplementation.INSTANCE.reloadReached1.acquire(); + ProjectStateData secondRound = mri.getLastCachedData(); + // new state is OK, but the previous should be obsoleted. + assertNotSame(secondRound, firstRound); + assertTrue(secondRound.isConsistent()); + assertFalse(firstRound.isConsistent()); + + Thread.sleep(2000); + Instant secondMod = Instant.now(); + Files.setLastModifiedTime(pomPath, FileTime.from(secondMod)); + + MonitoringReloadImplementation.INSTANCE.proceed.release(100); + + try { + ProjectState ps2 = newState.get(); + fail("Should report an exception"); + } catch (ExecutionException ex) { + assertTrue(ex.getCause() instanceof ProjectOperationException); + ProjectOperationException pex = (ProjectOperationException)ex.getCause(); + assertSame(ProjectOperationException.State.OUT_OF_SYNC, pex.getState()); + } + + ProjectState ps2 = ProjectReload.getProjectState(oci); + // may be valid or not, since Maven internally reloads the project independently, and + // will invalidate the state after load. + assertFalse(ps2.isConsistent()); + + assertTrue(ps2.getTimestamp() > baseMod.toEpochMilli()); + // will be at least the 1st modification + assertTrue(ps2.getTimestamp() >= firstMod.toEpochMilli()); + // and NOT the 2nd modification (modified twice -> error) + assertTrue(ps2.getTimestamp() < secondMod.toEpochMilli()); + + ProjectState ps3 = ProjectReload.withProjectState(oci, StateRequest.refresh()).get(); + assertTrue(ps3.isValid()); + assertTrue(ps3.isConsistent()); + assertTrue(ps3.getTimestamp() >= secondMod.toEpochMilli()); + } + + @ProjectServiceProvider(service = ProjectReloadImplementation.class, projectType = NbMavenProject.TYPE) + public static class MonitoringReloadImplementation implements ProjectReloadImplementation { + static MonitoringReloadImplementation INSTANCE; + final Semaphore proceed = new Semaphore(0); + final Semaphore reloadReached1 = new Semaphore(0); + final Semaphore reloadReached2 = new Semaphore(0); + volatile boolean enabled; + AtomicInteger counter = new AtomicInteger(); + + public MonitoringReloadImplementation() { + INSTANCE = this; + } + + + @Override + public CompletableFuture reload(Project project, StateRequest request, LoadContext context) { + if (!enabled) { + return null; + } + reloadReached1.release(); + try { + proceed.acquire(); + counter.incrementAndGet(); + } catch (InterruptedException ex) { + Exceptions.printStackTrace(ex); + } + reloadReached2.release(); + return null; + } + } }