A stopped job never comes back, and a replayed argv would re-download the mirror - #218
Merged
Merged
Conversation
onStopJob returned false, so a crawl the system stopped waited for the user. It now returns JobStopPolicy's verdict, and the argv a retry replays is rewritten so the engine resumes instead of downloading the mirror again. The engine raises HTS_CACHE_PRIORITY by itself when it finds hts-cache/hts-in_progress.lock, then lets the command line override it. Two Android options reach that field: -iC2 from the Update radio and -C0 from an unticked cache box. ResumeArgv.forStart drops both and puts one -iC1 at index 1, which is ahead of every URL. Position is the part that is not negotiable: an -iC* past the first URL sets argv_url to -1, and the engine then loads hts-cache/doit.log over the whole command line. Only a retry rewrites. A first attempt over an already-interrupted project is the user picking Continue or Update in the setup pane, and that choice has to stand, so the job stamps hts-cache/job-attempt.lock before it reaches the engine and every start the user asks for clears it. The engine's own lock cannot answer this: it says the mirror is resumable, not that this execution is the second one. No marker is added at crawl start, because the engine's lock plus our interrupted.lock already cover every window. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
forStart filters -iC* and -C* out and then inserts at index 1, so an argv whose
every token was dropped ({"-C0"}) leaves an empty list and out.add(1, ...)
throws IndexOutOfBoundsException. CrawlArgv.build always writes "httrack"
first, so no caller reaches it today, but a total function costs one Math.min.
ResumeArgv is also position-blind, which the filter loop now says: an engine
dashvalue_opt field whose value the user typed as -C0 is dropped as if it were
the option. Restructuring for that would need the engine's own argument
pre-pass, so it is recorded rather than fixed.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
Three defects the reviewers found in the retry path. Pressing Stop left the job armed. stopCrawl() reached MirrorJobService.cancel only where no owner answered, so a crawl the user stopped kept its scheduled job while the engine wound down. A system stop in that window returns JobStopPolicy.reschedules(), CrawlRun's finally has already stamped interrupted.lock, and the retry restarts a mirror the user cancelled. The cancel now sits outside the owner branches, under JobStopPolicy.cancelsScheduledJob(force, ownerAnswered). It is not literally unconditional, because a soft interrupt keeps the job. cancel() makes the system call onStopJob, which stops the engine hard, and that would cut the pending transfers the first press exists to let finish. The finished pane and the offline abandon both pass force, so both reach the cancel. A retry could be consumed by the wind-down of the crawl it was retrying. The next execution hits ProfileLockPolicy.alreadyInProgress, reports "already in progress", and its finally called jobFinished(params, false), abandoning the mirror. CrawlRun now records that refusal, and runCrawl reschedules on it. The reschedule fires only where the previous execution of this job was still running when this one started. That is what separates a wind-down from a genuine second crawl of the same project, which must stay refused rather than loop. An activity-owned crawl, or one the user started again, leaves the job's own slot empty or ended. The flag is read before the slot is overwritten, so it answers for the predecessor rather than for the new run. A wrong guess costs one extra retry, since the execution it names is the refused one, which ends at once. alreadyAttempted had no behavioural test. Returning false when the stamp exists and true after creating it kept everyUserStartClearsTheAttemptStamp green, because that guard counts occurrences and reads assignments. The inverted method overrides the user's own Continue or Update on a first attempt and re-downloads the mirror on a retry. It is pure java.io.File work, so it is now package-private and table-tested. The rows: no stamp, stamp present, stamp created then read, and a stamp that cannot be written at all. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
onStartJob builds a CrawlRun, and CrawlRun's engine field calls native init() as it is constructed. The libraries were only loaded later, on the crawl thread. A process that had already run HTTrackActivity survived, because onCreate had loaded them. A process the system started for the job alone died with UnsatisfiedLinkError. The system restarted the service, it crashed again, and after the third try the job was killed. That is the case stage 4 exists for, so the stage did nothing in its own scenario. Load the libraries in onStartJob, after setNotification so the ten-second deadline is untouched, and before the construction. A load that fails logs and returns false. That ends the job without a reschedule and leaves the crawl to the activity, the same answer schedule() gives when the system refuses the job. The freshProcess flag was dead code, because loadedSuccessfully() answers true before any load. So initRootPath never ran from the job and a fresh process had no emergency-log path. The flag now reads the new HTTrackLib.loadAttempted(), which is what the comment beside it always described. 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.
A stopped job did not come back.
onStopJobreturned false, so a crawl the system stopped waited for the user instead of continuing.It now returns
JobStopPolicy's verdict. A user stop from the Task Manager, an app cancel and a background restriction do not retry. Everything else does, timeouts included.The retry has to resume rather than start again, and that is the whole of this change. A rescheduled job replays its argv, and the engine's own resume detection loses to it.
hts-in_progress.locksetsHTS_CACHE_PRIORITYbefore the command line is parsed, and the parse then overrides it, so a replayed-C0or-iC2re-downloads the mirror.ResumeArgvdrops those tokens and forces-iC1at index 1, always ahead of the first URL. Position matters: an-iC*past the first URL leavesargv_urlat 0 and the engine spliceshts-cache/doit.logover the whole command line.Being interrupted is not enough to know a retry happened. A first attempt on a project holding a stale lock may be a deliberate Update, or a deliberate unticked cache. Forcing the continue mode there would override the user, while the pre-34 path would not. So the job stamps its own attempt marker, and clears it on every start the user asks for.
A retry in a fresh process used to crash before it resumed anything.
onStartJobbuilt aCrawlRun, whose engine field calls nativeinit()at once, while the libraries loaded later on the crawl thread. Only a process the system starts for the job alone was affected, which is why nothing caught it.origin/masterhas the same ordering, so this fixes an existing bug rather than one this stage introduced.493 tests pass, up from 467, and 19 mutations each turned a test red. On a device a killed crawl restarts once and reaches the engine in 270ms. It then fetches 234 paths, none of them fetched before the kill. 68 plus 234 is every path on the site, each fetched exactly once.
Part of #74, phase 3 stage 4 of 5.
Signed-off-by: Xavier Roche xroche@gmail.com
Co-authored-by: Claude Opus 5 (1M context) noreply@anthropic.com