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

Make middleware thread-safe #2

Merged
merged 1 commit into from
Jan 9, 2022
Merged

Make middleware thread-safe #2

merged 1 commit into from
Jan 9, 2022

Conversation

dentarg
Copy link
Member

@dentarg dentarg commented Jan 9, 2022

From tobmatth#105

Rack middleware must not use instance variables for state, as the same instance can be called by multiple threads. This patch changes the middleware to pass all request state as method arguments.

As a result of the lack of thread safety in the current version, it is possible for the middleware to handle a request incorrectly, using the @request instance variable from another request. This can present itself as random redirects to other URLs for a small subset of requests, when using a threaded Rack server like Puma.

I have reproduced and verified this issue using a test application with many requests in a specific pattern that we observed in production to trigger this issue. After this patch, the issue no longer occurs.

Credits to @titanous for these changes

Rack middleware must not use instance variables for state, as it is not
thread safe. This patch changes the middleware to pass all request state
as method arguments.
@dentarg dentarg merged commit c1cf7a1 into master Jan 9, 2022
@dentarg dentarg deleted the fix-thread-safety branch January 9, 2022 11:50
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.

2 participants