diff --git a/app/src/main/java/com/httrack/android/HTTrackActivity.java b/app/src/main/java/com/httrack/android/HTTrackActivity.java index 619ab89a..20bc4742 100755 --- a/app/src/main/java/com/httrack/android/HTTrackActivity.java +++ b/app/src/main/java/com/httrack/android/HTTrackActivity.java @@ -1431,6 +1431,9 @@ protected void runInternal() { RandomAccessFile outLock = null; FileLock lock = null; File profile = null; + // Did the engine get as far as running, and if so did it leave anything to resume? + boolean engineRan = false; + boolean pendingWork = true; try { // Sanity checks if (parent == null) { @@ -1496,7 +1499,9 @@ protected void runInternal() { setProgressLines(new String[] { string_starting_mirror }); // Run engine + engineRan = true; final int code = engine.main(cargs); + pendingWork = leavesPendingWork(interrupted || engine.wasStopped(), code); // Result if (code == 0) { @@ -1545,6 +1550,16 @@ protected void runInternal() { } catch (IOException io) { } } + // Stamp what the run actually was; a project the engine never touched keeps its marker. + if (engineRan) { + try { + setInterruptedProfile(pendingWork); + } catch (final IOException io) { + Log.w(getClass().getSimpleName(), "could not update the resume marker", io); + } + } + // Before the finished pane, whose own stopMirror() must not read as an interruption. + ended = true; } // Ensure we switch to the final pane @@ -1591,15 +1606,10 @@ public void run() { */ private synchronized void setInterruptedProfile(final boolean interrupted) throws IOException { - if (parent != null) { - if (interrupted) { - parent.setInterruptedProfile(true); - } else { - parent.setInterruptedProfile(false); - } - } else { + if (parent == null) { throw new IOException("parent has been detached"); } + parent.setInterruptedProfile(interrupted); } /* @@ -1622,12 +1632,14 @@ public boolean stopMirror(final boolean force) { } // Stop engine final boolean stopSent = engine.stop(force); - // If not yet stopped, mark as dirty - // ("Continue an interrupted mirror ...") - try { - setInterruptedProfile(stopSent); - } catch (final IOException io) { - Log.w(getClass().getSimpleName(), "could not lock file", io); + // Only a stop that lands on a live crawl leaves work behind: the finished pane asks for one + // too, and the engine answers it long after runInternal recorded the real outcome. + if (!ended) { + try { + setInterruptedProfile(true); + } catch (final IOException io) { + Log.w(getClass().getSimpleName(), "could not write the resume marker", io); + } } return stopSent; } @@ -1851,6 +1863,46 @@ protected boolean hasCacheFile() { return profile != null && profile.exists(); } + /** + * Does a crawl that ended this way leave the mirror resumable ? + * + * @param stopped + * Was the crawl cut short, by the user or by a cap the engine enforces itself ? + * @param engineCode + * The engine return code; nonzero means it gave up rather than finished. + * @return true if the project should reopen offering "Continue an interrupted download" + */ + protected static boolean leavesPendingWork(final boolean stopped, + final int engineCode) { + // Errors are not a criterion: the engine returns 0 once it has drained its queue, however + // many links failed on the way, and there is nothing left to continue. + return stopped || engineCode != 0; + } + + /** + * The marker a stopped or aborted crawl leaves behind. + * + * @param target + * The project directory + * @return The lock file + */ + protected static File getInterruptedLockFile(final File target) { + return new File(new File(target, "hts-cache"), "interrupted.lock"); + } + + /** + * Interrupted profile ? + * + * @param target + * The project directory + * @return true if the mirror was interrupted + */ + protected static boolean isInterruptedProfile(final File target) { + // The engine's own lock only outlives a crawl it never got to end, such as a killed process. + return new File(target, "hts-in_progress.lock").exists() + || getInterruptedLockFile(target).exists(); + } + /** * Interrupted profile ? * @@ -1858,12 +1910,27 @@ protected boolean hasCacheFile() { */ protected boolean isInterruptedProfile() { final File target = getTargetFile(); - if (target != null) { - final File cache = new File(target, "hts-cache"); - return new File(target, "hts-in_progress.lock").exists() - || new File(cache, "interrupted.lock").exists(); + return target != null && isInterruptedProfile(target); + } + + /** + * Set the "interrupted" flag. + * + * @param target + * The project directory + * @param interrupted + * Interrupted mirror ? + * @throws IOException + * Upon I/O error. + */ + protected static void setInterruptedProfile(final File target, + final boolean interrupted) throws IOException { + final File lock = getInterruptedLockFile(target); + if (interrupted) { + final FileWriter wr = new FileWriter(lock); + wr.close(); } else { - return false; + lock.delete(); } } @@ -1878,14 +1945,10 @@ protected boolean isInterruptedProfile() { protected synchronized void setInterruptedProfile(final boolean interrupted) throws IOException { final File target = getTargetFile(); - final File cache = new File(target, "hts-cache"); - final File lock = new File(cache, "interrupted.lock"); - if (interrupted) { - final FileWriter wr = new FileWriter(lock); - wr.close(); - } else { - lock.delete(); + if (target == null) { + throw new IOException("no project name defined!"); } + setInterruptedProfile(target, interrupted); } /** diff --git a/app/src/main/java/com/httrack/android/jni/HTTrackLib.java b/app/src/main/java/com/httrack/android/jni/HTTrackLib.java index 01ef9df9..b8b1667d 100755 --- a/app/src/main/java/com/httrack/android/jni/HTTrackLib.java +++ b/app/src/main/java/com/httrack/android/jni/HTTrackLib.java @@ -109,6 +109,14 @@ public static int buildTopIndex(final File path, final File templatesPath) { */ public native boolean stop(boolean force); + /** + * Was the last run cut short ? True for a stop() request, and for the size and time caps the + * engine enforces itself, which no return code of main() tells apart from a completion. + * + * @return true if the engine stopped before draining its queue + */ + public native boolean wasStopped(); + /** * Default constructor. */ diff --git a/app/src/main/jni/htslibjni.c b/app/src/main/jni/htslibjni.c index 3fcb6474..1122c9e4 100755 --- a/app/src/main/jni/htslibjni.c +++ b/app/src/main/jni/htslibjni.c @@ -827,6 +827,29 @@ Java_com_httrack_android_jni_HTTrackLib_stop(JNIEnv* env, jobject object, return stopped; } +JNICALL jboolean +Java_com_httrack_android_jni_HTTrackLib_wasStopped(JNIEnv* env, jobject object) { + HTTrackLib_context *const context = getNativeOpt(env, object); + jboolean stopped = JNI_FALSE; + + if (context == NULL) { + throwRuntimeException(env, "null context"); + return JNI_FALSE; + } + + MUTEX_LOCK(context->lock); + /* hts_main2() returns 0 for nearly every abort, so ask the engine instead. + stop is a cap or a user stop; exit_xh is a fatal disk error, a full link + table, an aborting callback, or a rolled-back session. */ + if (context->opt != NULL) { + stopped = context->opt->state.stop != 0 + || hts_is_exiting(context->opt) != 0 ? JNI_TRUE : JNI_FALSE; + } + MUTEX_UNLOCK(context->lock); + + return stopped; +} + static jint HTTrackLib_buildTopIndex(JNIEnv* env, jclass clazz, jstring opath, jstring otemplates) { if (opath != NULL && otemplates != NULL) { @@ -929,7 +952,7 @@ jint HTTrackLib_main(JNIEnv* env, jobject object, jobjectArray stringArray) { } /* Unreference global option tab */ - /* Nope - do this at destructor time */ + /* Freed at destructor time, not here: wasStopped() reads opt after main returns. */ /*MUTEX_LOCK(context->lock); hts_free_opt(context->opt); context->opt = NULL; diff --git a/app/src/main/jni/htslibjni.h b/app/src/main/jni/htslibjni.h index abb0125e..7dc7b73d 100644 --- a/app/src/main/jni/htslibjni.h +++ b/app/src/main/jni/htslibjni.h @@ -48,6 +48,9 @@ JNIEXPORT jboolean JNICALL Java_com_httrack_android_jni_HTTrackLib_stop(JNIEnv* env, jobject object, jboolean force); +JNIEXPORT jboolean JNICALL +Java_com_httrack_android_jni_HTTrackLib_wasStopped(JNIEnv* env, jobject object); + JNIEXPORT jint JNICALL Java_com_httrack_android_jni_HTTrackLib_buildTopIndex(JNIEnv* env, jclass clazz, jstring opath, jstring otemplates); diff --git a/app/src/test/java/com/httrack/android/InterruptedLockTest.java b/app/src/test/java/com/httrack/android/InterruptedLockTest.java new file mode 100644 index 00000000..a3008572 --- /dev/null +++ b/app/src/test/java/com/httrack/android/InterruptedLockTest.java @@ -0,0 +1,151 @@ +package com.httrack.android; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +/** hts-cache/interrupted.lock is what makes a project reopen on "Continue an interrupted + * download", so it has to mean the crawl was cut short and nothing else. */ +public class InterruptedLockTest { + @Rule + public final TemporaryFolder tmp = new TemporaryFolder(); + + private File target; + + @Before + public void setUp() throws Exception { + target = tmp.newFolder("project"); + new File(target, "hts-cache").mkdirs(); + } + + /** The engine returns 0 once its queue is drained, whatever failed on the way. */ + @Test + public void aRunThatReachesTheEndIsNotResumable() { + assertFalse("a crawl that ran to the end has nothing to continue", + HTTrackActivity.leavesPendingWork(false, 0)); + } + + @Test + public void aCrawlCutShortIsResumable() { + // A soft stop lets pending transfers finish, so the engine still returns 0; so does a size or + // time cap, which the engine applies by stopping itself. HTTrackLib.wasStopped() sees both. + assertTrue(HTTrackActivity.leavesPendingWork(true, 0)); + assertTrue(HTTrackActivity.leavesPendingWork(true, -1)); + } + + @Test + public void anEngineThatGaveUpIsResumable() { + assertTrue(HTTrackActivity.leavesPendingWork(false, -1)); + assertTrue(HTTrackActivity.leavesPendingWork(false, 1)); + } + + @Test + public void theMarkerRoundTrips() throws Exception { + assertFalse(HTTrackActivity.isInterruptedProfile(target)); + HTTrackActivity.setInterruptedProfile(target, true); + assertTrue(HTTrackActivity.isInterruptedProfile(target)); + HTTrackActivity.setInterruptedProfile(target, false); + assertFalse(HTTrackActivity.isInterruptedProfile(target)); + } + + /** The engine leaves this one behind only when it never got to end. */ + @Test + public void theEnginesOwnLockAlsoMeansInterrupted() throws Exception { + assertTrue(new File(target, "hts-in_progress.lock").createNewFile()); + assertTrue(HTTrackActivity.isInterruptedProfile(target)); + } + + /** What the finished pane does, end to end, for each way a crawl can end. */ + private boolean reopensOnContinue(final boolean stoppedByUser, final int engineCode) + throws IOException { + HTTrackActivity.setInterruptedProfile(target, + HTTrackActivity.leavesPendingWork(stoppedByUser, engineCode)); + return HTTrackActivity.isInterruptedProfile(target); + } + + @Test + public void everyOutcomeStampsTheProjectItsOwnWay() throws Exception { + assertFalse("a completed crawl", reopensOnContinue(false, 0)); + assertTrue("a stop, asked for or capped", reopensOnContinue(true, 0)); + assertTrue("an engine that gave up", reopensOnContinue(false, -1)); + assertFalse("a completed crawl again", reopensOnContinue(false, 0)); + } + + /** A project stopped last time and finished this time no longer offers to continue: the marker + * clears itself, which is all that ever clears one left by an older build. */ + @Test + public void aCleanRunClearsAStaleMarker() throws Exception { + HTTrackActivity.setInterruptedProfile(target, true); + assertFalse("a stale marker survived a clean run", reopensOnContinue(false, 0)); + } + + /** Runner.stopMirror needs a live AsyncTask, so the guard is pinned in the source instead. */ + private static String stopMirrorBody() throws IOException { + final String source = TestSources.javaSource("HTTrackActivity"); + // The last declaration is the runner's; the first is RunnerFragment's trunk to it. + final int from = source.lastIndexOf("public boolean stopMirror(final boolean force) {"); + assertTrue("Runner.stopMirror is gone", from != -1); + final int to = source.indexOf("\n }\n", from); + assertTrue("Runner.stopMirror is not closed where expected", to > from); + return source.substring(from, to); + } + + @Test + public void aStopRequestOnlyEverWritesTheMarker() throws Exception { + final Matcher m = Pattern.compile("setInterruptedProfile\\(([^)]*)\\)") + .matcher(stopMirrorBody()); + int calls = 0; + while (m.find()) { + calls++; + assertEquals("stopMirror must not clear the marker, nor pass a verdict it cannot make", + "true", m.group(1)); + } + assertEquals("stopMirror no longer touches the marker at all", 1, calls); + } + + @Test + public void aStopAfterTheCrawlEndedIsIgnored() throws Exception { + assertTrue("the finished pane's own stopMirror() would mark every project resumable", + stopMirrorBody().contains("if (!ended)")); + } + + /** The verdict belongs to the run, so it is written where the run ends. */ + @Test + public void theCrawlStampsItsOwnOutcome() throws Exception { + final String source = TestSources.javaSource("HTTrackActivity"); + final int from = source.indexOf("protected void runInternal()"); + final int to = source.indexOf("displayFinishedPanel(displayMessage, errorsCount,"); + assertTrue("runInternal no longer bounded by its displayFinishedPanel call", + from != -1 && to > from); + final String body = source.substring(from, to); + assertTrue("runInternal must weigh the engine's own stop, not just the user's", + body.contains("leavesPendingWork(interrupted || engine.wasStopped(), code)")); + assertTrue("runInternal must write the verdict before the finished pane opens", + body.contains("setInterruptedProfile(pendingWork)")); + assertTrue("ended must be set before the finished pane asks for a stop", + body.contains("ended = true")); + } + + /* The predicate below only ever sees what wasStopped() reports, so the engine's + two abort flags are pinned here: stop alone misses a fatal disk error. */ + @Test + public void wasStoppedWeighsBothOfTheEnginesAbortFlags() throws Exception { + final String jni = TestSources.jniSource("htslibjni.c"); + final int at = jni.indexOf("HTTrackLib_wasStopped"); + assertTrue("wasStopped is gone", at > 0); + final String body = jni.substring(at, jni.indexOf("\n}", at)); + assertTrue("wasStopped must read state.stop", body.contains("state.stop")); + assertTrue("wasStopped must also read exit_xh, which stop never sets", + body.contains("hts_is_exiting")); + } +}