diag: log temp-file removal for #722 (do not merge) - #749
Closed
PathGao wants to merge 3 commits into
Closed
Conversation
…earlier runs left A save writes a sibling temp file, fsyncs it and renames it over the document. On the machine in #722 a 0-byte `.doc.md.markpad-tmp-…` is left beside the document by every save, and the save itself SUCCEEDS: no error reaches the app, the tab's modified dot clears, and the document holds the new text. So the rename returned success and the source name is still in the directory afterwards — which is not a thing the code can cause, and not a thing that happens on a clean machine or in the reporter's VM. What is different there is an endpoint agent (亚信安全 TrustOne) sitting in the filesystem; auto-save is not involved, since turning it off changes nothing. Three changes, none of which needs to know what the agent is doing: The rename is followed by a check for the file it should have consumed, and removes it if it is there. On every other machine this is one `exists()` on a path that is gone, and it is the only place that knows the name at the moment it appears. The temp file's handle is now closed before the rename and before any cleanup, rather than at the end of the function. `MoveFileExW` and `DeleteFileW` fail with a sharing violation while a handle without `FILE_SHARE_DELETE` is open, and ours joined whatever a scanner was already holding on a file created an instruction ago; a delete that does get through with a handle open only marks the file delete-pending, so it stays in the directory meanwhile. The next save of a document also sweeps leftovers for that document that no live write could own: nothing under a minute old, then anything empty, and anything older than an hour. Empty and not-empty are separated because a temp file with contents can be the only copy of a document, from a process that died between the fsync and the rename, while an empty one can never be anything but garbage. Cleanup failures are also folded into the error the save returns instead of being dropped by `let _ =`, so the next report of this comes with the reason attached. ref #722
… startup The sweep a save performs reaches the folder of a document someone is still editing, and no other. A document that accumulated temp files and is then left alone keeps them for good — and the machine in #722 produces one per save, so a document put aside after an afternoon's work is exactly where a pile of them ends up. Startup asks for the documents Markpad already knows about: the recent list, and whatever the session restored. That is at most nine folders plus the open tabs, one `read_dir` each, off the main thread, with nothing on screen waiting for the result. The policy is the one the per-save sweep uses and it stays in one function, so the two cannot come to disagree about what a leftover is. Every window calls it and only the first pays: the sweep is once per document per run and Markpad's windows share a process. Not gated on the version upgrade. A marker file that says "cleaned once, at 2.7.7" is state to keep, to migrate and to get wrong, and what it would buy is skipping a `read_dir` on nine folders at startup. ref #722
Appends one line per rename, post-rename check, delayed re-check (200ms/2s/10s) and sweep removal to %TEMP%\markpad-diag.log, with the raw OS error code. Built for the reporter of #722 to run once.
Collaborator
Author
|
Served its purpose; log is on #722. |
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.
ref #722. Diagnostic build only, branched off #725. Writes what
renameand everyremove_fileon a temp file return, plus re-checks at 200ms/2s/10s, to%TEMP%\markpad-diag.log. Exists to get one log from the reporter's machine; close after that.