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

Thread safety #27

Closed
wants to merge 8 commits into from
Closed

Thread safety #27

wants to merge 8 commits into from

Conversation

pplant
Copy link

@pplant pplant commented Jul 9, 2020

Regarding the ticket I wrote concerning thread safety when setting proc_token_cookie lambada I created a PR solving the issue. The solution is based on how the ruby-i18n gem is solving the issue since the use case is more or less the same. A locale gets set on each request as the proc_token_cookie. Basically I added a Config class holding all the attribute accessors (hold by class << self). Keycloak extends a Base module that dynamically generates accessor functions forwarding and fetching values to the Config class. The Config class it self is initialized and written to the the thread variables. For each thread a new Config is initialized and hence we avoid having unwanted overrides and issues with running the gem within a multi threaded environment.

@kjoscha
Copy link

kjoscha commented Jan 13, 2021

hey @pplant thank you for the work! I am not maintaining this repo, but I want to use the gem in a most-up-to-date state. Do you think, your improvment is production-ready?

@kjoscha
Copy link

kjoscha commented Jan 13, 2021

I am working on some bugs by the way. For example, registering and passwort-reset function do not work in my dev environment

@pplant
Copy link
Author

pplant commented Jan 14, 2021

Hi @kjoscha. Yes I would say so, we were using my fork in production on multiple services for a few months. It definitely solves the issue with the thread safety. Since quite some months have passed creating the ticket and the pull request we decided to fork the project to our bitbucket repo and do a complete do over including the exchange of the http client gem (makes almost no difference while benchmarking), changed the overall structure of the gem to be more maintainable.

@Defoncesko
Copy link

Hi @kjoscha. Yes I would say so, we were using my fork in production on multiple services for a few months. It definitely solves the issue with the thread safety. Since quite some months have passed creating the ticket and the pull request we decided to fork the project to our bitbucket repo and do a complete do over including the exchange of the http client gem (makes almost no difference while benchmarking), changed the overall structure of the gem to be more maintainable.

@pplant why don't you share this new gem ?

@pplant
Copy link
Author

pplant commented May 18, 2021

Hi @kjoscha. Yes I would say so, we were using my fork in production on multiple services for a few months. It definitely solves the issue with the thread safety. Since quite some months have passed creating the ticket and the pull request we decided to fork the project to our bitbucket repo and do a complete do over including the exchange of the http client gem (makes almost no difference while benchmarking), changed the overall structure of the gem to be more maintainable.

@pplant why don't you share this new gem ?

Since this gem is not maintained anymore and I would not like to share something that I can't maintain. Furthermore the rewritting of the gem was done for the company I'm working, following serveral company specific changes that might add overhead for the open source community. But feel free to fork my fork, or create a patch from my PR to solve the thread safety issue.

@lvitals
Copy link
Member

lvitals commented Jun 18, 2024

The reported issue was fixed in PR #43

@lvitals lvitals closed this Jun 18, 2024
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.

4 participants