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

Potential thread safety issues #142

Closed
martinbertolino opened this issue Jan 18, 2022 · 3 comments
Closed

Potential thread safety issues #142

martinbertolino opened this issue Jan 18, 2022 · 3 comments

Comments

@martinbertolino
Copy link
Contributor

We have noticed that this middleware is randomly logging empty bodies for certain requests in our PROD environment. After doing some research and code reviews, I believe that there is a potential problem with the following members of the LoggingMiddleware class:

    self.boundary = ""
    self.cached_request_body = None

These two member can get mutated on a per call basis, however in a multi-threaded environment, they could be overwritten by a different thread before they are used for the current invocation.

The reason for this statement are the following sources:

https://blog.roseman.org.uk/2010/02/01/middleware-post-processing-django-gotcha/
https://stackguides.com/questions/10763641/is-this-django-middleware-thread-safe
https://stackoverflow.com/questions/6214509/is-django-middleware-thread-safe

I also did some testing on our code, and I have noticed that the middleware is sort of a 'singleton' that gets constructed only once per process and reused for all threads, which would explain the random empty bodies in our logs as the cached_request_body reference could be overwritten by a different thread.

I would be happy to craft and propose a solution and create a PR for it, but I have not seen much activity on this project so I want to check if it is still being maintained.

Let me know and I will propose a PR.

Thank you.

@famousfilm
Copy link
Contributor

@martinbertolino Thanks for noticing. Yes, feel free to put up an PR and ping me. I'll take a look.

@martinbertolino
Copy link
Contributor Author

martinbertolino commented Jan 22, 2022

@famousfilm PR has been created.

#143

@famousfilm
Copy link
Contributor

Fixed by #143

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

No branches or pull requests

2 participants