Closed
Conversation
in commit f20118a the call to fcgi_hash_init() moved out of the actual request handling to the point where FPM was only calling it once when it spawns a subprocess. Because the datastructure was not cleaned between handling multiple requests, headers and other data of previous requests were not cleaned correctly. This patch now calls fcgi_hash_clean() before handling individual requests at the cost of a bit of the performance gains of the previous commit.
Member
|
Would it be possible to add a test for this ? |
Contributor
Author
|
I would love to, if you could point me at some existing tests for FPM - it's currently unclear to me how FPM is being tested. |
Member
|
hmm, I already fixed it in 1d6228b thanks |
Contributor
Author
|
closing this one then. |
Contributor
Author
|
Question: Wouldn't my solution here be a slight bit better? After 1d6228b the request gets destroyed completely, but for some reason FPM wasn't doing that before - possibly for performance reasons. Just cleaning the request structure before handling the request seems less intrusive to me. Bear in mind though that I practically have no clue - just thinking aloud here. |
Contributor
Author
|
ah. right. Yeah - I see that |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
As I've said on the mailing list, I'm not 100% sure whether this is the correct usage of the fastcgi API, so I would really appreciate if somebody with fastcgi (and FPM) knowledge could give a review.
Furthermore, if somebody could give me some pointers in how to write tests for FPM, I will gladly provide tests for this too.
This fixes bug 70279:
in commit f20118a the call to fcgi_hash_init() moved out of the actual request handling to the point where FPM was only calling it once when it spawns a subprocess.
Because the datastructure was not cleaned between handling multiple requests, headers and other data of previous requests were not cleaned correctly.
This patch now calls fcgi_hash_clean() before handling individual requests at the cost of a bit of the performance gains of the previous commit.