Repository navigation
Remove at the next --deep scan the .fetch-* staging directories and cryptodeps-source-* caches left by scans stopped during a download, where flock is available (#70) - #71
Merged
Conversation
…ryptodeps-source-* caches left by scans stopped during a download, where flock is available (#70) A --deep fetch downloads into a .fetch-<random> staging directory in the source cache root and removes it in a deferred call. A scan stopped during the download by SIGINT, SIGTERM or SIGKILL never made that call, and no later scan looked at the directory. With a stub npm that sleeps, a scan sent SIGTERM (exit 143) or SIGINT (exit 130) left .fetch-<random> beside npm/ in ~/Library/Caches/cryptodeps, and it was still there after a second scan that ran to the end. A scan without a per-user cache directory left its cryptodeps-source-<random> cache in the temporary directory the same way, since only Close removes that cache. Each fetcher now takes a shared flock on the cache root before it stages anything there, and holds it until Close or CleanCache. The kernel lets go of it when the process exits, a SIGKILL included. At its first fetch into the cache, a fetcher that can take the lock exclusively, which no other fetcher in this process or another then holds, removes the .fetch-* directories in the root. It removes directories only: the cache entries, and a file or a link with the name, are left alone. The lock is on the root directory itself, so the cache root holds no new file. A scan's cache of its own is marked with a .locked file once its lock is held. A scan that makes its own cache then removes the cryptodeps-source-* directories beside it in the temporary directory that are private directories of the same user, carry the mark, and have their lock free. The lock and both removals are built where syscall.Flock exists: darwin, dragonfly, freebsd, illumos, linux, netbsd and openbsd. Elsewhere, Windows among them, no lock is taken and nothing is removed, as before. With the change, the second scan of the reproduction above leaves only npm/ in the per-user cache, and an empty temporary directory when there is no per-user cache directory, after SIGTERM and after SIGINT. The tests in internal/analyzer/source/cache_lock_flock_test.go start a scan in a process of its own and kill it with SIGKILL during a stub download. They check that a scan running beside it leaves its staging directory or its cache alone, and that the next scan removes it. They also check that a second fetcher in the same process leaves the first one's staging directory alone until the first one closes. Two more tests cover what the removals leave alone: cache entries, and files and links with the names, in the cache root; and unmarked, group-writable and differently named directories, a link and a file in the temporary directory. TestKilledScanLeavesNoStagingDirectoryForGood, TestKilledScanLeavesNoCacheOfItsOwnForGood and TestOnlyStagingDirectoriesAreRemovedFromTheRoot fail on the parent commit. What remains of #70 is Windows, where this change takes no lock and removes nothing. Refs #70
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.
This pull request lands 1 commit in internal/analyzer/source.
Included
Verified on this exact tree
go build ./...: exit 0go vet ./...: exit 0make lint(golangci-lint v1.64.8): exit 0, no findingsgo test -race -p 4 -count=1 -v ./...: exit 0; 13 packages ok, 3 packages with no test files; 549 top-level tests passed, 0 failed, 2 skipped (TestHelperFetchUntilKilled, TestIntegration_FetchPyPIPackage: "pip not available, skipping test"); 1664 passed, 0 failed, 2 skipped counting subtestsNot included
None
Closes
Refs #70
How to merge
Rebase and merge, so that each change stays its own commit on
main.