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

Request and Response Object can be deleted at every time #18

Closed
bzeller opened this issue Sep 5, 2013 · 3 comments
Closed

Request and Response Object can be deleted at every time #18

bzeller opened this issue Sep 5, 2013 · 3 comments

Comments

@bzeller
Copy link
Contributor

bzeller commented Sep 5, 2013

I openend this as a own issue. I came over it while stress testing the threading code
with autobench/httperf.
And i found 2 problems but they come both from the same source:

Httperf opens lots of connections at the same time and runs requests on the server.
Every request has a timeout and if the timeout is reached it just closes the socket.
This may be no problem with the non threaded version ( i didn't check yet) but in the
threaded version you have pending requests if the server load is high.
They are waiting in a queue, the problem is if they reach the connection timeout the
sockets are closed and deleted WHILE they are still waiting. Also we don't have
socket error handling code (a error maybe should emit some signal and close the
request).

This creates two problems:

  1. If you call flush on a closed socket you get a SIGPIPE termination. HttpFileHandler runs into this because it does not enter the eventloop when writing
    multiple times to the socket. This can maybe be fixed with using
    waitForBytesWritten() instead of flush as the docs state. This will return false
    if a error occured.

  2. The requests object are deleted behind the back of the application, i worked
    around this by using QPointer objects , so at least i know when they are gone.

The question is if we should think about a different approach than directly connecting
request objects to their deleteLater slot, so we can handle these things better.

@bzeller
Copy link
Contributor Author

bzeller commented Sep 6, 2013

After thinking about this some more, i would propose a change where we put the Request objects inside QSharedPointers, where as long as the request is valid the HttpServer holds a copy. When the request is
invalid because the connection was closed, the server removes that copy. The request object itself will
stay as long as there are other copies out in the wild.

On second thought that would mean a source compatibility break, because that shared object of course
needs to be used everywhere , also in usercode. But maybe we could create something like a shared
backend of HttpServerRequest ( HttpTcpRequestBackend, HttpFCGIRequestBackend) that goes invalid
when the request was closed.

To make that happen we of course would need some state and error checking code in HttpServerRequest:

HttpServerRequest::isValid or HttpServerRequest::isAlive
HttpServerRequest::hasError
HttpServerRequest::error

and so on.

@vinipsmaker
Copy link
Owner

Httperf opens lots of connections at the same time and runs requests on the server.
Every request has a timeout and if the timeout is reached it just closes the socket.
This may be no problem with the non threaded version ( i didn't check yet) but in the
threaded version you have pending requests if the server load is high.
They are waiting in a queue, the problem is if they reach the connection timeout the
sockets are closed and deleted WHILE they are still waiting. Also we don't have
socket error handling code (a error maybe should emit some signal and close the
request).

The threaded abstraction can disable the timeout while the request is queued.

  1. The requests object are deleted behind the back of the application, i worked
    around this by using QPointer objects , so at least i know when they are gone.

I see.

For simple applications there isn't any problem, but if you "store" the request objects, you is going to have troubles.

What do you think about this:

  1. Implement a new signal, HttpServerRequest::timeoutReached()
  2. HttpServer connects timeoutReached() to the QAbstractSocket::close() to maintain backwards compatibility.
  3. Complex applications can handle the timeout themself and the request object won't be deleted behind their back.

The question is if we should think about a different approach than directly connecting
request objects to their deleteLater slot, so we can handle these things better.

This is the simplest solution if we don't need special code to be triggered before deletion. The solution can be changed and I don't care, but what special code needs to be triggered before deletion?

@bzeller
Copy link
Contributor Author

bzeller commented Oct 10, 2013

I think this issue is gone with the fd-based thread code. Since in the handler thread the workflow is exactly
as it would be in a single thread application

@bzeller bzeller closed this as completed Oct 10, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants