Previously to prevent the user from deadlocking themselves in case the called `connection.stop()` from the `reconnecting` event we would first start the reconnecting process and then called the `reconnecting` event. This was racey since it was (very unlikely but still) possible that reconnect would finish quickly and would invoke `reconnected` callback before the `reconnecting` callback was invoked (happens once in a blue moon on a @BrennanConroy 's box). Another issue was that if the user called `connection.stop()` and `connection.start()` in the event the connection could be dropped so we would start a new reconnect attempt and as a result we would have 2 reconnecting attempts running in parallel but using the same disconnect cts and start event. The fix is to invoke the `reconnecting` event before starting the reconnecting process. We also copy the current disconnect cts which makes it possible to check if the connection was restarted in the `reconnecting` callback and if it was we are canceling this reconnect. This makes it impossible to have multiple concurrent reconnects.
We were signalling the `m_start_completed_event` after the connection reconnected but before we changed the connection state to `connected`. If there was an ongoing call to the `stop()` function the thread would get unblocked and could change the connection state to `disconnecting` before the thread doing the reconnect would have a chance to change the connection state to `connected`. This caused an assert because we assume that if reconnecting succeeded we should go from the `reconnecting` to the `connected` state. The fix is to signal the event after we changed the state. It is also correct because the connection has reconnected succesfully so the state it should be is the `connected` state.
`trace_log_writer` is the default log writer which (for windows) is using `OutputDebugString` to log the client activity. It was only public because it was used as the default parameter value for `log_writer` in the ctors of the `connection` and `hub_connection` classes. This class was not really meant to be used externally (and after one of the previous changes it could not even be used externally because the `write` method was not exported from the dll) so I am moving it from the public includes folder into the implementation folder.
- fixing paths to includes (how did it even compile before, especially on the CI?)
- `websockets` namespace was pulled out from the `experimental` namespace and renamed from `web_sockets`
- #ifdefing Windows specific stuff in stdafx.h
- Adding common 3rd party headers to stdafx.h to precompile them - makes build 25% faster
- Removing targetver.h - it was merged with stdafx.h
Reasons:
- secure websockets apparently did not work correctly in 2.2.0
- 2.4.0.1 has v140 compatible binaries we will need
Other changes:
- using headers installed with NuGet package and not with VS
- enforcing using the `std::min` function istead of the `min` macro defined only in a windows header file
- adding a public parameterless ctor to the `hub_proxy` class
- blocking passing `hub_connection` and `connection` instances by value to prevent from creating cycles where a `hub_connection` or `connection` instance is captured by value in a callback stored in the very same `hub_connection` or `connection` instance
Creating a targets file that will:
- Add the signalrclient include directory to %AdditionalIncludes%
- Add a correct version of signalrclient.lib to %AdditionalDependencies%
- Copy a correct version of signalrclient.dll to the project target directory after the project is built
The .targets file is added to the project by NuGet when installing the package
Remaining tasks
* add .props/.targets files so that the user does not have to add AdditionalIncludes/AdditionalDependencies manually
* review nuspec contents (links, licenses, etc.)
Adding the build.msbuild file to enable building and running tests from command line (now) and building the NuGet package (soon).
Renaming signalrclient.build.settings -> SignalRClient.Build.Settings to follow MSBuild conventions.
`connection_impl` allowed registering a callback when a message was received. This callback used to take a string as the parameter. However for hubs the message is in json format so we had to parse the parameter. Since SignalR uses json to communicate and the response was already parsed it is more efficient to pass the json object we already had instead of serializing it and re-parsing. To fix this we add an "overload" that allows registering a callback that takes `json::value` as the parameter. The way we do it is a bit clumsy (that's why this is "overload" and overload) since there isn't a straightforward way of adding and overload taking `std::function` with different parameters. Since the `connection_impl` is an internal class this should be fine - the public API does not reflect this in any way.
The change also fixes a corner case where a hub connection would receive a persistent connection message - before we would fail when parsing a primitive value, now we can handle this case gracefully.
Bonus:
- fixing a bug from a previous pull request where `connectionToken` and `connectionData` parameters were swapped
- adding a test that tests that the connection gets closed when `hub_connection_impl` goes out of scope
Xiaohongt
moozzyk
Pawel Kadluczka
BrennanConroy