gui-cs / Terminal.Gui

Cross Platform Terminal UI toolkit for .NET
MIT License
9.74k stars 694 forks source link

Some background operations are deceptively fragile and expensive #3228

Open dodexahedron opened 9 months ago

dodexahedron commented 9 months ago

In particular, this is referring to a small handful of methods which don't use async properly.

I'm fixing them in a dedicated branch based off of the current latest commit from @BDisp's constructor crusade branch, but I wanted to talk about the problems here as a reference to help avoid them in the future.

I will post the ones that I found (revealed to me thanks to the AsyncApostle ReSharper extension, which I have set to consider these cases to be errors) as individual issues in a project I have created on my fork to keep them together.

In general, they're cases of operations that are significantly more expensive than they look and which have high potential for permanent (for the life of the appdomain) breakage, deadlocks, and/or failure to continue processing.

The reasons for the problems aren't necessarily obvious or intuitive, but they're easy to fix and often can be done with less code and with greater efficiency and consistency at run-time and greater flexibility and extensibility, without any crazy tricks.

The project and associated issues are here: https://github.com/users/dodexahedron/projects/8

In git, the work is organized as a branch for the project, and then individual branches from that branch, per-issue, which will each be merged back to the project branch when completed, and then the project branch will be the one associated with the PR to Terminal.Gui's repo.

dodexahedron commented 9 months ago

Update: Once #3212 and associated work is merged, this will be re-branched from there.

Not going to rebase because it's more work and more error prone to deal with all the conflicts that would involve. I'll just stash my changes and apply them on the reformatted code.