Every finished crawl marked itself interrupted - #158
Merged
Conversation
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) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
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) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
Signed-off-by: Xavier Roche <roche@httrack.com> # Conflicts: # app/src/test/java/com/httrack/android/TestSources.java
xroche
added a commit
that referenced
this pull request
Aug 24, 2026
Review of the first commit found two defects in it. The abort branch sat ahead of the interrupted one, but exit_xh is not a sign that the engine gave up on its own: a stop early enough that nothing was saved is rolled back for want of data (htscore.c:2088, on a guard that never consults state.stop), and a forced stop refuses the loop callback (htscore.c:951/1072). Both of the commonest ways a user ends a crawl were about to be announced as aborts nobody asked for, undoing #158. The reason string could never arrive. _hts_errmsg has one writer, HTS_PANIC_PRINTF, whose call sites all sit in htscoremain.c ahead of the mirror and return -1, so main() returning 0 and a non-empty hts_errmsg() are disjoint. Every abort would have read "mirror aborted", and the escaping around it guarded text that cannot exist. So abortCode() hands back exit_xh itself and the three causes carry three messages, the full link table is dropped from the claim (htsparse.c returns -1 without touching exit_xh), and no engine string crosses JNI. Choosing the outcome moves to MirrorOutcome, which imports no android.* and is table-tested; both defects above fail it under mutation. between() moves to TestSources, where two tests had already inlined it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
xroche
added a commit
that referenced
this pull request
Aug 24, 2026
The review found the assertion I wrote to replace #158's guard was strictly weaker than the one it replaced. Rewriting the production line was fair, since stop != NONE is equivalent to the old expression, but contains("engine.wasStopped()") is satisfied by the ternary anywhere in runInternal, so leavesPendingWork(stop == Stop.USER, code) brought #158 back with both assertions green. The argument is now taken with balanced parentheses instead of up to the first comma, and it must read the run's verdict without narrowing it to one kind of stop. Changing false to Stop.ENGINE in two existing rows also left no row pairing Stop.NONE with a live abort code, so an abort that never set the stop flag could read as a success. Those rows are back alongside the new ones. Three more the same pass found: transposing the ternary's branches was invisible, so each branch is pinned to its source; the pane's first argument is pinned by position rather than by a four-character substring a comment could satisfy; and the new Stopped! wording was pinned to nothing. STOPPED_AT_LIMIT moves after the aborts, because declaration order read as a precedence and of() ranks it last. Stop.ENGINE's comment claimed the cap, which is only what of() decides later. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The finished pane calls
stopMirror(true)on the way in, to be sure a crawl that is still running gets stopped. The engine keeps its option block alive afterhts_main2returns, so that late request is always answered, and the answer went straight intohts-cache/interrupted.lock. Every crawl therefore ended by stamping itself interrupted, and every project the user had ever completed reopened defaulting to "Continue an interrupted download".The verdict now belongs to the run rather than to the stop request:
runInternalwrites the marker once, before the finished pane opens, and a stop that arrives after that is ignored. A crawl that drained its queue counts as finished however many links failed on the way. A user Stop, a nonzero engine code, and a size or time cap all count as cut short; the caps needed a newHTTrackLib.wasStopped(), because the engine applies them by stopping itself and still returns 0. Markers already on disk are left alone: nothing tells a stale one from a real interruption, so clearing them all would cost resumable state to anyone who genuinely did stop a crawl, and each one now corrects itself the next time that project is crawled to the end.