Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support cancellation of download checksums #3778

Merged
merged 1 commit into from
Feb 17, 2023

Conversation

HebaruSan
Copy link
Member

Problem

  1. Choose to install a large mod on Windows (so the validation step will take long enough that you have to wait for it)
  2. Apply changes
  3. Wait till the download is finished and the ZIP is being validated (you'll see the progress bar's label change)
  4. Click cancel during the validation step
  5. Apply changes again
  6. The file will re-validate, probably with its progress bar flickering back and forth; wait till it finishes
  7. An exception is thrown:
Unhandled exception:
System.IO.IOException: The process cannot access the file 'C:\Users\danny\AppData\Local\CKAN\downloads\downloading\E4B2C9E4-Parallax-StockTextures-2.0.0.zip' because it is being used by another process.
   at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
   at System.IO.File.InternalDelete(String path, Boolean checkHost)
   at CKAN.NetAsyncModulesDownloader.ModuleDownloadComplete(Uri url, String filename, Exception error, String etag)
   at CKAN.NetAsyncDownloader.FileDownloadComplete(Int32 index, Exception error, Boolean canceled, String etag)
   at CKAN.NetAsyncDownloader.<>c__DisplayClass20_0.<DownloadModule>b__1(Object sender, AsyncCompletedEventArgs args, String etag)
   at System.Net.WebClient.OnDownloadFileCompleted(AsyncCompletedEventArgs e)
   at CKAN.ResumingWebClient.OnOpenReadCompleted(OpenReadCompletedEventArgs e)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem()
   at System.Threading.ThreadPoolWorkQueue.Dispatch() 

Cause

The validation step doesn't support cancellation (even though we included it in the low-level ComputeHash extension function in #3659), so when the second install attempt starts, the original validation is still happening, and another one starts up in parallel. Then one thread (probably the first one that the user tried to cancel) finishes and tries to copy the temp file to the cache and delete it. But the other thread is still validating the temp file, so the deletion attempt throws an exception (on Windows, which locks files that are in use).

Changes

Now if the user clicks cancel while a mod is being validated, validation will stop before the file is copied to cache and deleted, so future install attempts can proceed without being blocked by another thread.

The file will be left in the in-progress download folder because there's most likely nothing wrong with it, and if there is something wrong, it will be caught in any future validation attempts.

Fixes #3777.

Known limitations

NetFileCache.ZipValid can't be cancelled, because ZipFile.TestArchive can't be cancelled. So if the user tries to cancel while this is in progress, it will finish that step, and then the cancellation happens afterwards before the SHA1 check starts. This should still be fine since the deletion attempt will no longer occur.

(The SHA1 and SHA256 checks are both immediately cancellable.)

@HebaruSan HebaruSan added Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN Pull request Windows Issues specific for Windows labels Feb 16, 2023
Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That all makes sense, good investigation and explanation. LGTM

@HebaruSan HebaruSan force-pushed the fix/cancel-validation branch from 457e0d8 to 38f7023 Compare February 17, 2023 04:28
@HebaruSan HebaruSan merged commit 56e4217 into KSP-CKAN:master Feb 17, 2023
@HebaruSan HebaruSan deleted the fix/cancel-validation branch February 17, 2023 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN Windows Issues specific for Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple threads validating same download simultaneously after one was cancelled
2 participants