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

Add article on threading #2568

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Conversation

tznind
Copy link
Collaborator

@tznind tznind commented Apr 21, 2023

Often we encounter users who are using Threads but do not know about Application.MainLoop.Invoke. For example:

Yes every one sec I have to use the same frameView with new data.

#2562 (comment)

I have created an article to explain how to update main UI thread from a background task.

Note that this targets develop so current users should see it. We can cherry pick it into v2_develop

Pull Request checklist:

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

@tznind
Copy link
Collaborator Author

tznind commented Apr 21, 2023

I see though that there is a section in mainloop.md but I think its worthy to be its own article since it is so common to see this issue.

@migueldeicaza
Copy link
Collaborator

migueldeicaza commented Apr 22, 2023

This has been on the docs for a very long time:

https://github.com/gui-cs/Terminal.Gui/blob/develop/docfx/articles/mainloop.md#threading

Oh, I see your second comment now.

Yeah, might make sense to give it more visibility, given this

@@ -0,0 +1,51 @@
# Threading
It is common for a developer to run one or more background tasks on a separate
`Thread` (or `Task`). For example to periodically fetch data. If you want to
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use the "infinitive form" in API docs instead of the "impersonal you". The infinitive form of a verb is its basic, uninflected form, typically preceded by the word "to" in English. For example, "to eat," "to play," or "to learn." By using the infinitive form, the sentence becomes more concise and focuses on the action itself.

This would be rewritten as "To make changes to a View from a background task, issue the change on the main Terminal.Gui Thread thread."

Miguel tends to write using the "impersonal you" form, so a lot of the old docs still use this. But I've worked hard to rid most API docs of it. Thanks.

@En3Tho
Copy link
Contributor

En3Tho commented May 7, 2023

I guess it's worth mentioning that Termital.Gui has it's own SynchronizationContext and it really depends on how this task is started.

(Note: SynchronizationContext is just an abstraction over some kind of Scheduler. In case of Terminal.Gui it's a single UI thread message loop)

Few examples with default System.Threading.Task:

Button clicked by user
 -> Processed by UI thread
 -> Has SynchronizationContext
 -> SomeAsyncMethodCalled with no ConfigureAwait(false)
 -> Some awaits inside
 -> Always jump back to UI's thread SynchronizationContext when await is done
 -> UI updates are valid

In this case some Task is started from UI thread and SynchronizationContext is saved so technically it's not background.
Button clicked by user
 -> Processed by UI thread
 -> Has SynchronizationContext
 -> SomeAsyncMethodCalled with ConfigureAwait(false)
 -> Some awaits inside
 -> Always jump to thread pool because ConfigureAwait(false) makes it ignore SynchronizationContext
 -> UI updates are invalid

In this case some Task is started from UI thread but SynchronizationContext is not saved so technically it's background.
Some work started with Task.Run
 -> Processed by ThreadPool thread
 -> No SynchronizationContext
 -> ...
 -> UI updates are invalid

This case is quite straight: no SynchronizationContext from the start because it's queued directly on ThreadPool

Examples above show that the problem is that when using async machinery you can't always be sure whether you are on UI thread or not.

Possible means to fix this:

  1. Do every UI update via Appication.MainLoop (like in the doc) if you're not sure code is running on UI thread after an await.
  2. (My favorite) Just write a custom Task that will always run on UI thread via Mainloop.Invoke and run every async UI update using this custom Task. This is very similar to 1) but is done automatically

I hope this helps someone understand it better

@tig
Copy link
Collaborator

tig commented May 10, 2023

@tznind did you see my change request?

@tznind
Copy link
Collaborator Author

tznind commented May 10, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants