Skip to content

Fix thread safety issues with RestClient when used as a single instance for Authentication and Cookies #1795

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

Closed

Conversation

kendallb
Copy link
Contributor

@kendallb kendallb commented Mar 16, 2022

Description

Implements the following changes:

  • BaseUrl needs to be internal so it cannot be changed after the client is constructed by the user.
  • ResponseWriter and AdvancedResponseWriter are back to regular property setters (no longer init)
  • Moved handling of Cookies out of HttpClient and into RestSharp, so they will not cross pollinate requests.
  • Make the CookieContainer a property on the request, not the client.
  • Move Authenticator from the client into the request, since it's request specific.
  • Add tests for cookie handling.
  • Add back all non-sync functions to RestClient and extensions

Purpose

Fix issues referenced about thread safety with single instance and improve the API to help developers avoid causing those problems.

  • Bugfix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

…er properties. This means any code attempting to consume RestSharp and use these features needs to update their toolchain to support C# 9.0 or the code will not compile. We still have a lot of developers using Visual Studio 2019, which does not support C# 9.0. While we are migrating to VS2022, it's a process to get everyone moved over.

Please change these back to regular setters so they can be used with earlier versions of Visual Studio.
…y will not cross pollinate requests.

Make the CookieContainer a property on the request, not the client.
Move Authenticator from the client into the request, since it's request specific.
Add tests for cookie handling.
…so, as they may vary by request and we do not want folks messing with the shred single instance of RestClient.
…d at the request level, not at the client level. There are many valid reasons to change this on a per request basis if a client is shared.
@kendallb kendallb force-pushed the feature/kendallb/thread-safety-and-more branch from 037fc31 to 16608d1 Compare March 17, 2022 18:00
… via async not sure how to convert that one).
@kendallb
Copy link
Contributor Author

Gonna close this out out and I can cherry pick bits into separate PR's for you and you can merge them. I will rebase this working tree as they get accepted.

@kendallb kendallb closed this Mar 18, 2022
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.

1 participant