Thread safety #16
Open
Thread safety #16
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds a global mutex (which is not optimal, but for loads a server handling dynamic content manageable) and a per-pool cache for the resolved path. The document root is set per request so no cleanup is needed anymore. For sake of simplicity we decided not to got with double buffering and removing the mutex was not worth it for now.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Looking at #4 I wanted to take a stab at adding thread safety, because I didn't see a reason why it should not be possible to get it working.
Context / Why are we making this change?
When using Apache with mod_proxy for PHP-FPM or any other scripting language, threaded MPMs are a good fit. This pull request adds thread safety.
Testing and QA Plan
Reproducing the existing issues way easy enough (simply by using ab with high enough request concurrency). The code changes have been reviewed carefully and tested with the same scripts that triggered the errors before.
Impact
For any threaded and non-threaded MPM, there is an overhead because of the newly introduced mutex. In our test, it was still possible to achieve 16k req/s even with a prefork MPM on a single workstation.
The added thread safety and broader deployment range is worth it IMHO.