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

Multiple manual downloads, uncached filter, purge option #2930

Merged
merged 2 commits into from
Nov 25, 2019

Conversation

HebaruSan
Copy link
Member

Problem

When you click the Download button in the mod info pane or right click a row and choose Download Contents, the GUI switches to the progress tab and begins downloading the module. The tabstrip is not locked down.

If you switch back to the mod list and initiate a second manual download while the first is still in progress, CKAN crashes with an exception like this:

System.InvalidOperationException: This BackgroundWorker is currently busy and cannot run multiple tasks concurrently.
  at System.ComponentModel.BackgroundWorker.RunWorkerAsync (System.Object argument) [0x00008] in <3310c48c8d57467d8dc60ad91e20e6c6>:0 
  at (wrapper remoting-invoke-with-check) System.ComponentModel.BackgroundWorker.RunWorkerAsync(object)
  at CKAN.MainModInfo.StartDownload (CKAN.GUIMod module) [0x00039] in <5434f55888ed4862bbdac3b97deafc6a>:0 
  at (wrapper remoting-invoke-with-check) CKAN.MainModInfo.StartDownload(CKAN.GUIMod)
  at CKAN.Main.downloadContentsToolStripMenuItem_Click (System.Object sender, System.EventArgs e) [0x00012] in <5434f55888ed4862bbdac3b97deafc6a>:0 
  at System.Windows.Forms.ToolStripItem.OnClick (System.EventArgs e) [0x00019] in <ef4c141df2a64a07af6c70f96a472f03>:0 
  at System.Windows.Forms.ToolStripMenuItem.OnClick (System.EventArgs e) [0x00090] in <ef4c141df2a64a07af6c70f96a472f03>:0 
  at System.Windows.Forms.ToolStripMenuItem.HandleClick (System.Int32 mouse_clicks, System.EventArgs e) [0x00000] in <ef4c141df2a64a07af6c70f96a472f03>:0 
  at System.Windows.Forms.ToolStripItem.FireEvent (System.EventArgs e, System.Windows.Forms.ToolStripItemEventType met) [0x00054] in <ef4c141df2a64a07af6c70f96a472f03>:0 
  at (wrapper remoting-invoke-with-check) System.Windows.Forms.ToolStripItem.FireEvent(System.EventArgs,System.Windows.Forms.ToolStripItemEventType)
  at System.Windows.Forms.ToolStrip.OnMouseUp (System.Windows.Forms.MouseEventArgs mea) [0x00048] in <ef4c141df2a64a07af6c70f96a472f03>:0 
  at System.Windows.Forms.ToolStripDropDown.OnMouseUp (System.Windows.Forms.MouseEventArgs mea) [0x00000] in <ef4c141df2a64a07af6c70f96a472f03>:0 
  at System.Windows.Forms.Control.WmLButtonUp (System.Windows.Forms.Message& m) [0x00078] in <ef4c141df2a64a07af6c70f96a472f03>:0 
  at System.Windows.Forms.Control.WndProc (System.Windows.Forms.Message& m) [0x001b4] in <ef4c141df2a64a07af6c70f96a472f03>:0 
  at System.Windows.Forms.ScrollableControl.WndProc (System.Windows.Forms.Message& m) [0x00000] in <ef4c141df2a64a07af6c70f96a472f03>:0 
  at System.Windows.Forms.ToolStrip.WndProc (System.Windows.Forms.Message& m) [0x00000] in <ef4c141df2a64a07af6c70f96a472f03>:0 
  at System.Windows.Forms.ToolStripDropDown.WndProc (System.Windows.Forms.Message& m) [0x00017] in <ef4c141df2a64a07af6c70f96a472f03>:0 
  at System.Windows.Forms.Control+ControlWindowTarget.OnMessage (System.Windows.Forms.Message& m) [0x00000] in <ef4c141df2a64a07af6c70f96a472f03>:0 
  at System.Windows.Forms.Control+ControlNativeWindow.WndProc (System.Windows.Forms.Message& m) [0x0000b] in <ef4c141df2a64a07af6c70f96a472f03>:0 
  at System.Windows.Forms.NativeWindow.WndProc (System.IntPtr hWnd, System.Windows.Forms.Msg msg, System.IntPtr wParam, System.IntPtr lParam) [0x00085] in <ef4c141df2a64a07af6c70f96a472f03>:0 

Cause

Downloading is done by a BackgroundWorker, which can only handle one task at a time. When you start the second download, CKAN attempts to tell its worker to start up again while it's already active, which throws the exception.

Changes

  • NetAsyncDownloader is refactored to allow new downloads to be added to an in-progress group of downloads
  • The cURL code is removed from NetAsyncDownloader because it makes the class harder to maintain, and we have not lately solved any user problems by suggesting the "chicken bits" environment variable (I have not attempted to remove all of the cURL code from everywhere)
  • NetAsyncModulesDownloader is updated to exclude modules already being downloaded when starting new downloads, in case the user chooses Download Contents for the same module multiple times in a row
  • MainModInfo.ContentsDownloadButton_Click and Main.downloadContentsToolStripMenuItem_Click now both call MainModInfo.StartDownload, which avoids trying to run the background worker if it's already busy and instead asks NetAsyncModulesDownloader to add them to the current batch
  • The background worker's input and output are now GUIMods instead of CkanModules so their cached status can be updated properly after completion

Minor stuff:

  • MainModInfo._PostModCaching no longer calls RecreateDialogs because this isn't needed and has nothing to do with downloading modules
  • MainModInfo.CacheWorker is removed because this is private state that's not appropriate to share
  • A variety of misnamed variables are renamed for consistency with other variables

To make this more convenient to test:

  • A new Uncached filter is added to GUI, which is simply the inverse of the Cached filter
  • A new Purge Contents right click menu option is added to the mod list grid, which removes all downloads for the selected identifier when clicked (including previous/incompatible versions)

Note that if you open the Cached filter and Purge Contents for some mod, it remains in the list, and the same goes for opening the Uncached filter and clicking Download Contents. I considered updating the filters after caching, but I think that might be confusing and annoying instead of helpful since a mod that you might have intended to work with further would disappear. The right click menu options should toggle their enabled status appropriately, though.

Fixes #1680.
Fixes #1713.
Fixes #2882.

@HebaruSan HebaruSan added Bug Something is not working as intended GUI Issues affecting the interactive GUI Core (ckan.dll) Issues affecting the core part of CKAN Pull request Network Issues affecting internet connections of CKAN labels Nov 24, 2019
@HebaruSan HebaruSan requested a review from DasSkelett November 24, 2019 22:57
@HebaruSan HebaruSan added the Enhancement New features or functionality label Nov 25, 2019
@DasSkelett
Copy link
Member

I don't understand the comment of Download():

        /// <summary>
        /// Downloads our files, returning an array of filenames that we're writing to.
        /// The sole argument is a collection of DownloadTargets.
        /// The .onCompleted delegate will be called on completion.
        /// </summary>

returning an array of filenames that we're writing to.
The function is a void and doesn't return anything, what is that supposed to mean? Was that once the case and the description hasn't been updated since then?

@DasSkelett
Copy link
Member

Can we update the the Contents tab of the metadata panel after purging a mod from the cache?
For caching it does so in the _PostModCaching() callback: UpdateModContentsTree(SelectedModule.ToCkanModule(), true);

Maybe move this into GUIMod.UpdateIsCached()?

@HebaruSan
Copy link
Member Author

Took care of those in latest commit.

Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

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

Thanks!

@HebaruSan HebaruSan merged commit a7f2bed into KSP-CKAN:master Nov 25, 2019
@HebaruSan HebaruSan deleted the fix/multi-downloads branch November 26, 2019 18:28
@DasSkelett DasSkelett added the i18n Issues regarding the internationalization of CKAN and PRs that need translating label Jan 18, 2021
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 Enhancement New features or functionality GUI Issues affecting the interactive GUI i18n Issues regarding the internationalization of CKAN and PRs that need translating Network Issues affecting internet connections of CKAN
Projects
None yet
2 participants