diff --git a/src/main/java/com/conveyal/r5/analyst/NetworkPreloader.java b/src/main/java/com/conveyal/r5/analyst/NetworkPreloader.java index c3ee89aef..1d3f904b8 100644 --- a/src/main/java/com/conveyal/r5/analyst/NetworkPreloader.java +++ b/src/main/java/com/conveyal/r5/analyst/NetworkPreloader.java @@ -97,7 +97,10 @@ public LoaderState preloadData (AnalysisWorkerTask task) { * data is prepared. */ public TransportNetwork synchronousPreload (AnalysisWorkerTask task) { - return buildValue(Key.forTask(task)); + Key key = Key.forTask(task); + TransportNetwork scenarioNetwork = buildValue(key); + setComplete(key, scenarioNetwork); + return scenarioNetwork; } @Override @@ -140,7 +143,7 @@ protected TransportNetwork buildValue(Key key) { linkedPointSet.getEgressCostTable(progressListener); } } - // Finished building all needed inputs for analysis, return the completed network to the AsyncLoader code. + // Finished building all needed inputs for analysis, return the completed network return scenarioNetwork; } diff --git a/src/main/java/com/conveyal/r5/util/AsyncLoader.java b/src/main/java/com/conveyal/r5/util/AsyncLoader.java index 4606803bb..9a49d85ec 100644 --- a/src/main/java/com/conveyal/r5/util/AsyncLoader.java +++ b/src/main/java/com/conveyal/r5/util/AsyncLoader.java @@ -28,7 +28,7 @@ * "value is present in map". * * Potential problem: if we try to return immediately saying whether the needed data are available, - * there are some cases where preparing the reqeusted object might take only a few hundred milliseconds or less. + * there are some cases where preparing the requested object might take only a few hundred milliseconds or less. * In that case then we don't want the caller to have to re-poll. In this case a Future.get() with timeout is good. * * Created by abyrd on 2018-09-14 @@ -123,9 +123,7 @@ public LoaderState get (K key) { setProgress(key, 0, "Starting..."); try { V value = buildValue(key); - synchronized (map) { - map.put(key, new LoaderState(Status.PRESENT, null, 100, value)); - } + setComplete(key, value); } catch (Throwable t) { // It's essential to trap Throwable rather than just Exception. Otherwise the executor // threads can be killed by any Error that happens, stalling the executor. @@ -139,7 +137,8 @@ public LoaderState get (K key) { /** * Override this method in concrete subclasses to specify the logic to build/calculate/fetch a value. - * Implementations may call setProgress to report progress on long operations. + * Implementations may call setProgress to report progress on long operations; if they do so, any callers of this + * method are responsible for also calling setComplete() to ensure loaded objects are marked as PRESENT. * Throw an exception to indicate an error has occurred and the building process cannot complete. * It's not entirely clear this should return a value - might be better to call setValue within the overridden * method, just as we call setProgress or setError. @@ -155,6 +154,12 @@ public void setProgress(K key, int percentComplete, String message) { } } + public void setComplete(K key, V value) { + synchronized (map) { + map.put(key, new LoaderState(Status.PRESENT, "Loaded", 100, value)); + } + } + /** * Call this method inside the buildValue method to indicate that an unrecoverable error has happened. * FIXME this will permanently associate an error with the key. No further attempt will ever be made to create the value.