Implement non blocking cgi #362
Merged
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
…s also the PID of the child process
… go back where we read the buffer with the response and we send it
|
@552020 It does not compile according to GitHub actions Ubuntu |
… possible 500 from the CGI
…GI don't wait and read the CGI output but pass the fd of the pipe to the resoponse object
…ildResponse/buildCGIResponse
…m-multiple-sockets tests(Dockerfile): upload Dockerfile for multi socket and upload Dock…
dantol29
requested changes
May 17, 2024
|
@lmangall Stefano and I made it work!! Now we have proper CGI timeout and it is non-blocking! |
dantol29
approved these changes
May 21, 2024
We were testing in the main therefore it was failing. But I didn't test the branch, soon to be main. |
|
Nice, great job ! |
|
Did you test with the "parallel CGI" test ? |
I tried but we have some issues: https://linear.app/web-serv/issue/WEB-330/cgi-bug-in-main-%F0%9F%9A%A8 |
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.
If you want to take a look at the code, this is a first draft, it compiles, and most of the logic is there.
The only part is missing is when the CGI returns successfully (or not). We will not wait in executeCGI anymore but we will wait in startPollEventLoop: the wait in executeCGI has not been removed yet. After the CGI returned successfully in startPollEventLoop, where we wait for all processes (-1) without blocking (WNOHANG), we go back to poll, the event we listen for for that connection will be POLLOUT. And then we need a flag like CGIhasFinished. We already collect the exitStatus of the CGI which is set in the Connection object.
The code to read the buffer written by the CGI is in handleConnection > buildResponse > routeRequest > handleRequest > executeCGI. We should re-enter handleConnection and buildResponse, but skip the whole logic we already executed the first time we entered it to execute the CGI. Alternatively we could also pass the fd of the pipe when we execute _connection.addCGI(pid); but this is dirtier.
I don't know if you @lmangall has capacity to think about it. I guess with a flag like _CGIhasTerminated (which means it has been executed or kill) we could skip all the unnecessary part of the code, and come to the point where we check the status and read from the buffer. Probably the first step should be just split executeCGI in two function: executeCGI and processExecutedCGI (or something like this), where we read from the buffer or depending on the status code we send back some error.