From 8f542d5cb900415d7ed339a878e86d62362dce93 Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Sun, 23 Aug 2026 11:46:54 +0200 Subject: [PATCH 1/2] Every finished crawl marked itself interrupted The finished pane stops the runner on the way in, and the engine answers a stop long after hts_main2 has returned, so interrupted.lock was written after every crawl and every completed project reopened on "Continue an interrupted download". Decide the marker from the run instead: runInternal writes it once, before the finished pane opens, and a stop that arrives afterwards is ignored. Errors do not count as unfinished work. A user stop and a nonzero engine code do, and so do the size and time caps, which the engine applies by stopping itself while still returning 0, so HTTrackLib.wasStopped() reports them. Markers already on disk are left in place; nothing tells a stale one from a real interruption, and each corrects itself on the next completed crawl. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Xavier Roche --- .../com/httrack/android/HTTrackActivity.java | 113 ++++++++++---- .../com/httrack/android/jni/HTTrackLib.java | 8 + app/src/main/jni/htslibjni.c | 21 +++ app/src/main/jni/htslibjni.h | 3 + .../httrack/android/InterruptedLockTest.java | 138 ++++++++++++++++++ 5 files changed, 258 insertions(+), 25 deletions(-) create mode 100644 app/src/test/java/com/httrack/android/InterruptedLockTest.java diff --git a/app/src/main/java/com/httrack/android/HTTrackActivity.java b/app/src/main/java/com/httrack/android/HTTrackActivity.java index b5581e92..196e84a2 100755 --- a/app/src/main/java/com/httrack/android/HTTrackActivity.java +++ b/app/src/main/java/com/httrack/android/HTTrackActivity.java @@ -1376,6 +1376,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) { @@ -1441,7 +1444,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) { @@ -1490,6 +1495,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 @@ -1536,15 +1551,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); } /* @@ -1567,12 +1577,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; } @@ -1796,6 +1808,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 ? * @@ -1803,12 +1855,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(); } } @@ -1823,14 +1890,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 2c3d1100..243accee 100755 --- a/app/src/main/jni/htslibjni.c +++ b/app/src/main/jni/htslibjni.c @@ -785,6 +785,27 @@ 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); + /* The engine raises this on its own when a size or time cap cuts the mirror + short, which no return code of hts_main2() distinguishes from a completion. */ + if (context->opt != NULL) { + stopped = context->opt->state.stop != 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) { 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..57f9e97a --- /dev/null +++ b/app/src/test/java/com/httrack/android/InterruptedLockTest.java @@ -0,0 +1,138 @@ +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")); + } +} From 0cbe516e04455380cd784eb5a7dad95f93911787 Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Sun, 23 Aug 2026 15:39:39 +0200 Subject: [PATCH 2/2] Ask the engine about both of its abort flags, not one Review caught a regression this PR introduced. wasStopped() read state.stop, which only a cap or a user Stop sets, and hts_main2 returns 0 for nearly every other abort. A mirror killed by a full disk, by a full link table, by an aborting callback or rolled back for want of a connection therefore looked finished, and the marker was deleted rather than merely not written. Under master it was always written, so those users would have lost a resume they used to have and re-downloaded the site. It now also asks hts_is_exiting(), which is exit_xh and is what the engine sets in all four cases. The unit tests could not see this: they drive the predicate with hand-supplied pairs, so they re-assert what I believed the engine does. The new one reads the JNI source and fails if wasStopped stops consulting either flag. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Xavier Roche --- app/src/main/jni/htslibjni.c | 10 ++++++---- .../com/httrack/android/InterruptedLockTest.java | 13 +++++++++++++ .../test/java/com/httrack/android/TestSources.java | 5 +++++ 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/app/src/main/jni/htslibjni.c b/app/src/main/jni/htslibjni.c index 243accee..ea608ca9 100755 --- a/app/src/main/jni/htslibjni.c +++ b/app/src/main/jni/htslibjni.c @@ -796,10 +796,12 @@ Java_com_httrack_android_jni_HTTrackLib_wasStopped(JNIEnv* env, jobject object) } MUTEX_LOCK(context->lock); - /* The engine raises this on its own when a size or time cap cuts the mirror - short, which no return code of hts_main2() distinguishes from a completion. */ + /* 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 ? JNI_TRUE : JNI_FALSE; + stopped = context->opt->state.stop != 0 + || hts_is_exiting(context->opt) != 0 ? JNI_TRUE : JNI_FALSE; } MUTEX_UNLOCK(context->lock); @@ -907,7 +909,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/test/java/com/httrack/android/InterruptedLockTest.java b/app/src/test/java/com/httrack/android/InterruptedLockTest.java index 57f9e97a..81304f83 100644 --- a/app/src/test/java/com/httrack/android/InterruptedLockTest.java +++ b/app/src/test/java/com/httrack/android/InterruptedLockTest.java @@ -135,4 +135,17 @@ public void theCrawlStampsItsOwnOutcome() throws Exception { 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(); + 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")); + } } diff --git a/app/src/test/java/com/httrack/android/TestSources.java b/app/src/test/java/com/httrack/android/TestSources.java index 02e30808..5afca3d1 100644 --- a/app/src/test/java/com/httrack/android/TestSources.java +++ b/app/src/test/java/com/httrack/android/TestSources.java @@ -63,6 +63,11 @@ static String javaSource(final String name) throws IOException { + ".java")); } + /** Source of the JNI glue, for contracts no JUnit test can reach at runtime. */ + static String jniSource() throws IOException { + return read(new File(dir("src/main/jni"), "htslibjni.c")); + } + /** A file of the pinned engine submodule, such as "winprofile-keys.tsv". */ static File engineFile(final String name) { return new File(dir("src/main/jni/httrack"), name);