Geetha Manjunath wrote: > > Hmm.. immediately there is one additional question on clientReadRequest > > when called from icapReqModReadReply. How does it handle the case where > > the full request could not be read immediately? To me it looks like the > > connection gets stuck in such case with clientReadRequest installed as a > > read handler for the ICAP connection, but nobody taking care of kicking > > the requests going again.. > > I am not sure I understand your question fully.. please explain... If > you mean "continue after being defered", then the fact that > commSetDefer() is not used on icap_fd before calling clientReadRequest > from icapReqModReadReply unlike a similar call from httpAccept() > probably does the trick.
No, I am not talking about deferred reads, I am talking about the situation where the modified HTTP request returned from the ICAP server is fragmented into multiple packets and cannot be immediately read in full by the call from The way icapReqModReadReply() calls clientReadRequest() seems to assume clientReadRequest will be able to immediately read and process the request, but there is no guarantee the full modified request is yet available. If the full modified request is not yet available clientReadRequest() installs itself as a read handler for the filedescriptor (well, it always does this), and returns without having a request attached to the connState. icapReqModReadReply() then continues and finds no request, only destroying the icap state.. (not sure if this also closes the ICAP filedescriptor). Lets assume this does not close the ICAP filedescriptor. comm_poll() then finds that more data is now available and calls clientReadRequest() to have it read. clientReadRequest() reads the request and attaches it to the connState, but there is nothing in clientReadRequest() which initiates continued processing of the request. You only took away the call to clientAccessCheck(), you did not add another call to notify the icap code. The correct design would be icapReqModReadReply reading the ICAP response headers, then hand off to clientReadRequest for reading the modified request, and finally have clientReadRequest call back into the icap code to continue processing of the request instead of calling clientAccessCheck(). Alternatively you can have icapReqModReadReply verifying that the modified HTTP request is available in the TCP buffer before calling clientReadRequest() but such design is not clean and will cause problems if the modified request is larger than the TCP receive window/buffer. In detail the case I am worring about is icapReqModReadReply() called with ICAP response headers + part of the modified HTTP request received icapReqModReadReply reads the ICAP response headers, and makes call to clientReadRequest for reading the modified HTTP request. clientReadRequest finds that the request is partial and does not process it yet, only reads what is available, and tells comm_poll to wait for more data to arrive on the filedescriptor and then call clientReadRequest() again. This is the purpose of the commSetSelect() call in clientReadRequest(). control returns to icapReqModReadReply before the full modified request has been received by clientReadRequest. Regards Henrik
