Migrate from using Threads to using Tasks for receiving and sending data #26
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Very nice, thank you! Will have a look when I'm back from vacation. |
paviad
added a commit
to paviad/MarcusW.VncClient
that referenced
this pull request
Jul 26, 2024
paviad
added a commit
to paviad/MarcusW.VncClient
that referenced
this pull request
Jul 26, 2024
paviad
added a commit
to paviad/MarcusW.VncClient
that referenced
this pull request
Jul 26, 2024
paviad
added a commit
to paviad/MarcusW.VncClient
that referenced
this pull request
Jul 26, 2024
paviad
added a commit
to paviad/MarcusW.VncClient
that referenced
this pull request
Jul 28, 2024
paviad
added a commit
to paviad/MarcusW.VncClient
that referenced
this pull request
Jul 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes the background tasks from using threads to instead use tasks.
The reason for this change is that using Threads using ThreadStart will keep the thread reserved for that background job, even though that job might just wait for input. That means no-one else can use that thread which might lead to depleting the thread pool if you had multiple VNC connections open. It thus isn't very scalable.
The second benefit of not using threads is that exceptions can't be handled in threads at all, which leads to unhandleable problems like in #25.
The only issue with my implementation is that any exceptions thrown get silently eaten until the Task is awaited (which in my case happens when
StopSendLoopAsync()is called). I don't really know of a good solution for this, however feedback is welcome. It might also be small enough to be considered a "won't fix™".If you have any questions about my implementation, please reach out.
Closes #25