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

Deal with threading and download errors #2374

Merged
merged 1 commit into from
Mar 19, 2018

Conversation

HebaruSan
Copy link
Member

Problems

As of #2351, I'm getting these errors when trying to install a mod on Windows:

System.ArgumentException: Controls created on one thread cannot be parented to a control on a different thread.
   at System.Windows.Forms.Control.ControlCollection.Add(Control value)
   at System.Windows.Forms.ToolStripControlHost.SyncControlParent()
   at System.Windows.Forms.ToolStripControlHost.OnParentChanged(ToolStrip oldParent, ToolStrip newParent)
   at System.Windows.Forms.ToolStripItem.set_ParentInternal(ToolStrip value)
   at System.Windows.Forms.ToolStrip.SetDisplayedItems()
   at System.Windows.Forms.StatusStrip.SetDisplayedItems()
   at System.Windows.Forms.ToolStrip.OnLayout(LayoutEventArgs e)
   at System.Windows.Forms.StatusStrip.OnLayout(LayoutEventArgs levent)
   at System.Windows.Forms.Control.PerformLayout(LayoutEventArgs args)
   at System.Windows.Forms.ToolStrip.DoLayoutIfHandleCreated(ToolStripItemEventArgs e)
   at System.Windows.Forms.ToolStrip.OnItemVisibleChanged(ToolStripItemEventArgs e, Boolean performLayout)
   at System.Windows.Forms.ToolStripItem.OnVisibleChanged(EventArgs e)
   at System.Windows.Forms.ToolStripItem.SetVisibleCore(Boolean visible)
   at System.Windows.Forms.ToolStripControlHost.SetVisibleCore(Boolean visible)
   at CKAN.Main.ShowWaitDialog(MainCancelCallback cancelAction)
   at CKAN.Main.InstallMods(Object sender, DoWorkEventArgs e)
   at System.ComponentModel.BackgroundWorker.OnDoWork(DoWorkEventArgs e)
   at System.ComponentModel.BackgroundWorker.WorkerThreadStart(Object argument)

Of course this works fine on Linux. Three cheers for "write once run anywhere."

If a downloaded file doesn't pass validation (invalid ZIP, wrong file size or hashes), a few problems happen:

  • The error message doesn't name the mod with the problem, which makes it harder for the user to deal with it
  • The error message is displayed by the uncaught exceptions handler, so the GUI ends up in an inconsistent state or crashes outright
  • Other mods in the same download batch may be discarded instead of being added to the cache, even if they are valid

Causes

WinForms on Windows only allows its GUI controls to be accessed from certain threads; other threads have to call Control.Invoke to pass code to be run on the appropriate thread. I assume this has to do with it being a wrapper around code from the 1990s, because if you were designing a GUI framework from scratch, you'd probably design it to not care about this sort of thing.

That's why Util.Invoke is sprinkled throughout the GUI code. The above exception happened on this line:

StatusProgress.Visible = true;

... which was being called from a background thread without Invoke.

Changes

Now Util.Invoke calls are added to all affected functions in MainWait.cs. The status bar progress bar is in a separate block because I found instances during investigation where putting it in the same Invoke as the regular progress bar wasn't good enough.

InvalidModuleFileKraken is now caught and displayed to the user in NetAsyncModulesDownloader.ModuleDownloadsComplete, which allows the loop to finish caching all the downloaded mods.

The mod name and version are added to the text for InvalidModuleFileKraken.

image

@HebaruSan HebaruSan added Bug Something is not working as intended GUI Issues affecting the interactive GUI Pull request and removed Bug Something is not working as intended labels Mar 18, 2018
@politas politas merged commit 6fd4287 into KSP-CKAN:master Mar 19, 2018
politas added a commit that referenced this pull request Mar 19, 2018
@politas politas removed Bug Something is not working as intended Pull request labels Mar 19, 2018
@HebaruSan HebaruSan deleted the fix/threading-exceptions branch March 19, 2018 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI Issues affecting the interactive GUI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants