Write the emergency dump somewhere it can actually land#13
Open
xroche wants to merge 1 commit into
Open
Conversation
emergencyDump() passed Environment.getExternalStorageState() to new File(). That returns the status string, "mounted", so the dump went to the relative path mounted/HTTrack/error.txt, resolved against the process working directory of /, where FileWriter threw straight into an empty catch. It has never written a file, on any release. The catch now logs, since silence is what hid this. Being static, it had no Context to ask for a directory, which is likely how it drifted; it takes the Runner's parent activity instead, and writes into our own external directory, falling back to the internal one. A crash path must not depend on a permission or on a volume being mounted, and neither does that. The JNI glue had the same hole: assertion failures fell back to a hardcoded /mnt/sdcard/Download/HTTrack/error.txt, a layout gone since KitKat, whose fopen has been failing quietly ever since. Once initRootPath() has run, emergencyLog already points into the mirror root; before it, there is nowhere we may write, and logcat has the assertion either way. MKDIR_MODE went with it, having no users. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| fclose(dumpFile); | ||
| /* Nothing is writable before initRootPath(); logcat above carries it either way. */ | ||
| if (emergencyLog != NULL) { | ||
| FILE *const dumpFile = fopen(emergencyLog, "wb"); |
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
2 participants
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.
emergencyDump()passedEnvironment.getExternalStorageState()intonew File(). That returns the status string,"mounted", so the dump aimed at the relative pathmounted/HTTrack/error.txt, resolved against the process working directory of/, whereFileWriterthrew straight into an emptycatch. It has never written a file, on any release. The catch now logs, since the silence is what hid it for nine years.Being
static, it had noContextto ask for a directory, which is plausibly how it drifted there in the first place. It now takes theRunner's parent activity and writes into our own external directory, falling back to the internal one. A crash path should not depend on a permission or on a volume being mounted, and this one no longer does.The JNI glue had the same hole from the other side: assertion failures fell back to a hardcoded
/mnt/sdcard/Download/HTTrack/error.txt, a storage layout gone since KitKat, whosefopenhas failed quietly ever since. OnceinitRootPath()has run,emergencyLogalready points into the mirror root; before it, there is nowhere we may legally write, and logcat carries the assertion either way.MKDIR_MODEwent with it, having had no users.Demonstrated rather than argued, since the failure is invisible by construction:
No tracker issue matches these symptoms in either repo, so there is nothing to close. Independent of the storage PRs, and compatible with either root. The new path needs a device to confirm end to end, which is not available yet.