Fix network recovery #64
Merged
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
Fix a number of problems when handling disconnections, and with asynchronous tasks in general. When `AbstractProtocol.AutoRecover` is set to true: * requests sent through the network would never be resolved when the connection is lost before they receive a response. This means that clients awaiting for a response would freeze forever. With this PR, those tasks are correctly rejected with a `ConnectionLost` exception (we cannot queue them: as far as the SDK is concerned, it cannot know if these requests were received and processed by Kuzzle or not) * there was a race condition when triggering the threaded recovering process where, in rare cases, 2 reconnection attempts would run in parallel, doubling the number of opened sockets * Race conditions could occur and corrupt the requests cache, or freeze the SDK because of an unhandled exception, because of an incorrect use of `lock` (this keyword has no impact when multiple accesses are made to the same "locked" object within the same thread) => semaphores are now used to circumvent the problem * Requests tasks were incorrectly configured, making their resolution synchronous. This means that resolving tasks in the SDK event handlers (which manage network state changes) could prevent them to finish in a timely fashion (or... ever) because they would synchronously trigger awaiting code * Save our standard code style directly in the solution to share it to whomever works on the project
Codecov Report
@@ Coverage Diff @@
## 2-dev #64 +/- ##
==========================================
+ Coverage 82.02% 82.52% +0.49%
==========================================
Files 34 34
Lines 1675 1722 +47
Branches 197 201 +4
==========================================
+ Hits 1374 1421 +47
+ Misses 277 276 -1
- Partials 24 25 +1
Continue to review full report at Codecov.
|
Merged
scottinet
added a commit
that referenced
this pull request
Sep 29, 2020
# [2.0.5](https://github.com/kuzzleio/sdk-csharp/releases/tag/2.0.5) (2020-09-29) #### Bug fixes - [ [#64](#64) ] Fix network recovery ([scottinet](https://github.com/scottinet)) ---
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.
Description
Fix a number of problems when handling disconnections, and with asynchronous tasks in general.
Network disconnection
When
AbstractProtocol.AutoRecoveris set to true:ConnectionLostexception (we cannot queue them: as far as the SDK is concerned, it cannot know if these requests were received and processed by Kuzzle or not)Asynchronous tasks
lock(this keyword has no impact when multiple accesses are made to the same "locked" object within the same thread) => semaphores are now used to circumvent the problemBoyscout