close_connection on RequestHandler causes process hang with CTRL-C

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

close_connection on RequestHandler causes process hang with CTRL-C

jslowery@gmail.com

Ever since I have gotten cherrypy3, I've had problems with the
cherrypy process hanging in the shell after a CTRL-C is sent to
process. My only resort would be to send a SIGTERM or SIGKILL.

I was looking around in the code and came up with this:

When CTRL-C is sent to to the cherrypy process, each connection thread
with an active connection waits until until a request does not return
a body or close_connection on the request is set to True (wsgiserver/
__init__.py:611) . The forever loop here implements HTTP/1.1 and Keep-
Alive. with the close_connection attribute on the HTTPRequest.

Therefore, there is a noticable time lag between when a
KeyboardInterrupt is sent and all of the worker threads are able to
process their SHUTDOWN request (because conn.communicate is blocking)
wsgiserver/__init__.py:693.

While the loop is spinning on the Connection's communicate method,
sending another CTRL-C to the process causes the thread to abort and
the callstack bubble up out of the thread join. At this point, the
only recourse is to manually SIGTERM.

It would appear that since the shutdown requests are put in a worker
queue, there currently isn't any way to give the HTTPConnection a flag
to say stop processing requests.

I suppose that the best thing to do would allow the KeyBoardInterrupts
to not lock the process during the shutdown process.

A fix I came up with is adding KeyboardInterrupt to the thread join
loop in wsgiserver/__init__.py near line 986

Go from

   while self._workerThreads:
            worker = self._workerThreads.pop()
            if worker is not current and worker.isAlive:
                try:
                    worker.join()
                except AssertionError:
                    pass

to

   while self._workerThreads:
            worker = self._workerThreads.pop()
            if worker is not current and worker.isAlive:
                try:
                    worker.join()
                except (AssertionError, KeyboardInterrupt):
                    pass

This at least allows the process to shutdown gracefully after multiple
CTRL-C's.



To reproduce this situation (test system Ubuntu Feisty):

test file:
#broke.py
import cherrypy

class Root(object):
    @cherrypy.expose
    def index(self):
        return 'hi'

cherrypy.quickstart(Root())

$python broke.py
<open localhost:8080 in web browser to activate a connection>
tap CTRL-C twice.


However, I wish that I could figure out how to interrupt the
connection loop when a shutdown is occuring, since Keep-Alive could
keep the server in the shutdown state for an arbitrarily long time
while multiple requests from the same client. However,


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "cherrypy-devel" group.
To post to this group, send email to [hidden email]
To unsubscribe from this group, send email to [hidden email]
For more options, visit this group at http://groups.google.com/group/cherrypy-devel
-~----------~----~----~----~------~----~------~--~---

Reply | Threaded
Open this post in threaded view
|

Re: close_connection on RequestHandler causes process hang with CTRL-C

Robert Brewer

jlowery wrote:

> Ever since I have gotten cherrypy3, I've had problems with the
> cherrypy process hanging in the shell after a CTRL-C is sent to
> process. My only resort would be to send a SIGTERM or SIGKILL.
>
> I was looking around in the code and came up with this:
>
> When CTRL-C is sent to to the cherrypy process, each connection thread
> with an active connection waits until until a request does not return
> a body or close_connection on the request is set to True (wsgiserver/
> __init__.py:611) . The forever loop here implements HTTP/1.1 and Keep-
> Alive. with the close_connection attribute on the HTTPRequest.
>
> Therefore, there is a noticable time lag between when a
> KeyboardInterrupt is sent and all of the worker threads are able to
> process their SHUTDOWN request (because conn.communicate is blocking)
> wsgiserver/__init__.py:693.
>
> While the loop is spinning on the Connection's communicate method,
> sending another CTRL-C to the process causes the thread to abort and
> the callstack bubble up out of the thread join. At this point, the
> only recourse is to manually SIGTERM.
>
> It would appear that since the shutdown requests are put in a worker
> queue, there currently isn't any way to give the HTTPConnection a flag
> to say stop processing requests.
>
> I suppose that the best thing to do would allow the KeyBoardInterrupts
> to not lock the process during the shutdown process.
>
> A fix I came up with is adding KeyboardInterrupt to the thread join
> loop in wsgiserver/__init__.py near line 986
>
> Go from
>
>    while self._workerThreads:
>             worker = self._workerThreads.pop()
>             if worker is not current and worker.isAlive:
>                 try:
>                     worker.join()
>                 except AssertionError:
>                     pass
>
> to
>
>    while self._workerThreads:
>             worker = self._workerThreads.pop()
>             if worker is not current and worker.isAlive:
>                 try:
>                     worker.join()
>                 except (AssertionError, KeyboardInterrupt):
>                     pass
>
> This at least allows the process to shutdown gracefully after multiple
> CTRL-C's.
>
>
>
> To reproduce this situation (test system Ubuntu Feisty):
>
> test file:
> #broke.py
> import cherrypy
>
> class Root(object):
>     @cherrypy.expose
>     def index(self):
>         return 'hi'
>
> cherrypy.quickstart(Root())
>
> $python broke.py
> <open localhost:8080 in web browser to activate a connection>
> tap CTRL-C twice.

Excellent work! I've been aware of this for some time and just haven't
had the time to really nail it down. Thanks for the patch!

> However, I wish that I could figure out how to interrupt the
> connection loop when a shutdown is occuring, since Keep-Alive could
> keep the server in the shutdown state for an arbitrarily long time
> while multiple requests from the same client. However,

I think you're talking about the "while" loop inside
HTTPConnection.communicate? That usually waits on rfile.readline()
inside parse_request, and *that* is waiting on
socket._fileobject._sock.recv(). I wonder what happens if we call
rfile._sock.close() in the main thread while another thread is waiting
on recv()?


Robert Brewer
System Architect
Amor Ministries
[hidden email]

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "cherrypy-devel" group.
To post to this group, send email to [hidden email]
To unsubscribe from this group, send email to [hidden email]
For more options, visit this group at http://groups.google.com/group/cherrypy-devel
-~----------~----~----~----~------~----~------~--~---

Reply | Threaded
Open this post in threaded view
|

Re: close_connection on RequestHandler causes process hang with CTRL-C

jslowery@gmail.com

Thanks

> I think you're talking about the "while" loop inside
> HTTPConnection.communicate? That usually waits on rfile.readline()
> inside parse_request, and *that* is waiting on
> socket._fileobject._sock.recv(). I wonder what happens if we call
> rfile._sock.close() in the main thread while another thread is waiting
> on recv()?
>

Well, I tested this out. .close() doesn't work, but shutdown() does.
I'm still learning the threading internals of cherrypy, so my solution
here may have thread-related side-effects. I also think this fix is
kind of kludgy. Maybe you know a better way to fit it into the design.

All of these changes are in wsgiserver/__init__.py

Modify the WorkerThread class thusly (minus my debug print lines of
course):

class WorkerThread(threading.Thread):
    """Thread which continuously polls a Queue for Connection objects.

    server: the HTTP Server which spawned this thread, and which owns
the
        Queue and is placing active connections into it.
    ready: a simple flag for the calling server to know when this
thread
        has begun polling the Queue.

    Due to the timing issues of polling a Queue, a WorkerThread does
not
    check its own 'ready' flag after it has started. To stop the
thread,
    it is necessary to stick a _SHUTDOWNREQUEST object onto the Queue
    (one for each running WorkerThread).
    """
    conn = False

    def __init__(self, server):
        self.ready = False
        self.server = server
        threading.Thread.__init__(self)

    def run(self):
        try:
            self.ready = True
            while True:
                conn = self.conn = self.server.requests.get()
                #print 'handling request on thread %s with conn %s' %
\
                #(self, conn)
                if conn is _SHUTDOWNREQUEST:
                    return

                try:
                    #print 'thread %s communicating' % self
                    conn.communicate()
                    #print 'thread %s end communicate' % self
                finally:
                    conn.close()
        except (KeyboardInterrupt, SystemExit), exc:
            self.server.interrupt = exc


In the CherrypyWSGIServer:stop method, change:

     # Must shut down threads here so the code that calls
        # this method can know when all threads are stopped.
        for worker in self._workerThreads:
            self.requests.put(_SHUTDOWNREQUEST)



to

     # Must shut down threads here so the code that calls
        # this method can know when all threads are stopped.
        for worker in self._workerThreads:
            self.requests.put(_SHUTDOWNREQUEST)
            if worker.conn and not worker.conn.rfile.closed:
 
worker.conn.rfile._sock.shutdown(_socket.SHUT_RDWR)


also need to import _socket in the top imports.


One side effect of this is that it is possible for the socket calls
under the communicate() method to throw a bad file descriptor
exception. I was able to generate one, but it did not hang the process
and everything shut down properly. (Just got an exception occured in
WorkerThread message).

In effect, this kills the worker thread dead in its tracks.

Hitting CTRL-C stops the process instantly, although this should
probably be tested on win32 as winsock can be a PITA sometimes.




--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "cherrypy-devel" group.
To post to this group, send email to [hidden email]
To unsubscribe from this group, send email to [hidden email]
For more options, visit this group at http://groups.google.com/group/cherrypy-devel
-~----------~----~----~----~------~----~------~--~---

Reply | Threaded
Open this post in threaded view
|

Re: close_connection on RequestHandler causes process hang with CTRL-C

Robert Brewer

jlowery wrote:

> > I think you're talking about the "while" loop inside
> > HTTPConnection.communicate? That usually waits on rfile.readline()
> > inside parse_request, and *that* is waiting on
> > socket._fileobject._sock.recv(). I wonder what happens if we call
> > rfile._sock.close() in the main thread while another thread
> > is waiting on recv()?
>
> Well, I tested this out. .close() doesn't work, but shutdown() does.
> I'm still learning the threading internals of cherrypy, so my solution
> here may have thread-related side-effects. I also think this fix is
> kind of kludgy. Maybe you know a better way to fit it into the design.
>
> ...
>
> # Must shut down threads here so the code that calls
> # this method can know when all threads are stopped.
> for worker in self._workerThreads:
>     self.requests.put(_SHUTDOWNREQUEST)
>     if worker.conn and not worker.conn.rfile.closed:
>         worker.conn.rfile._sock.shutdown(_socket.SHUT_RDWR)
>
> One side effect of this is that it is possible for the socket calls
> under the communicate() method to throw a bad file descriptor
> exception. I was able to generate one, but it did not hang the process
> and everything shut down properly. (Just got an exception occured in
> WorkerThread message).
>
> In effect, this kills the worker thread dead in its tracks.
>
> Hitting CTRL-C stops the process instantly, although this should
> probably be tested on win32 as winsock can be a PITA sometimes.

I can test on win32...I'll try to fold this into test_states so we can
test on multiple platforms more easily, and get the regression benefits.

We should give the deployer a "server.shutdown_timeout" config
entry--setting it to zero makes SIGTERM or Ctrl-C stop the process
instantly, setting it to a positive float gives existing connections N
seconds to complete before being forcibly shut down. That should allow a
reasonable way to shutdown gracefully--if you need immediate forced
shutdown, there's always SIGKILL.


Robert Brewer
System Architect
Amor Ministries
[hidden email]

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "cherrypy-devel" group.
To post to this group, send email to [hidden email]
To unsubscribe from this group, send email to [hidden email]
For more options, visit this group at http://groups.google.com/group/cherrypy-devel
-~----------~----~----~----~------~----~------~--~---

Reply | Threaded
Open this post in threaded view
|

Re: close_connection on RequestHandler causes process hang with CTRL-C

jslowery@gmail.com

> We should give the deployer a "server.shutdown_timeout" config
> entry--setting it to zero makes SIGTERM or Ctrl-C stop the process
> instantly, setting it to a positive float gives existing connections N
> seconds to complete before being forcibly shut down. That should allow a
> reasonable way to shutdown gracefully--if you need immediate forced
> shutdown, there's always SIGKILL.
>

Makes sense. For the timeout mechanism. Do you think the best way
would be to write a Shutdown thread that is launched in stop() that
time.sleep() before killing all of the sockets?


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "cherrypy-devel" group.
To post to this group, send email to [hidden email]
To unsubscribe from this group, send email to [hidden email]
For more options, visit this group at http://groups.google.com/group/cherrypy-devel
-~----------~----~----~----~------~----~------~--~---

Reply | Threaded
Open this post in threaded view
|

Re: close_connection on RequestHandler causes process hang with CTRL-C

Robert Brewer

jlowery wrote:

> > We should give the deployer a "server.shutdown_timeout" config
> > entry--setting it to zero makes SIGTERM or Ctrl-C stop the process
> > instantly, setting it to a positive float gives existing
> connections N
> > seconds to complete before being forcibly shut down. That
> should allow a
> > reasonable way to shutdown gracefully--if you need immediate forced
> > shutdown, there's always SIGKILL.
> >
>
> Makes sense. For the timeout mechanism. Do you think the best way
> would be to write a Shutdown thread that is launched in stop() that
> time.sleep() before killing all of the sockets?

worker.join takes a timeout arg. Let's move this conversation to
http://www.cherrypy.org/ticket/691.


Robert Brewer
System Architect
Amor Ministries
[hidden email]

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "cherrypy-devel" group.
To post to this group, send email to [hidden email]
To unsubscribe from this group, send email to [hidden email]
For more options, visit this group at http://groups.google.com/group/cherrypy-devel
-~----------~----~----~----~------~----~------~--~---

Reply | Threaded
Open this post in threaded view
|

Re: close_connection on RequestHandler causes process hang with CTRL-C

jslowery@gmail.com
In reply to this post by jslowery@gmail.com

Ok, well here is a thread class for wsgiserver/__init__.py


class SocketShutdownMonitor(threading.Thread):
    """ Waits a specific time interval before forcefully shutting down
    all of the worker threads. """
    def __init__(self, workers, *args, **kwargs):
        self.workers = list(workers)
        self.timeout = cherrypy.config.get("server.shutdown_timeout",
3)

        # Anything to keep it out of a tight loop
        self.shutdown_poll_interval = 0.1

        threading.Thread.__init__(self, *args, **kwargs)

    def run(self):
        start_time = time.time()
        # Allow workers to shut themselves down properly
        while self.workers and (time.time() - start_time) <
self.timeout:
            idx = 0
            for worker in self.workers:
                if not worker.conn or worker.conn.rfile.closed:
                    del self.workers[idx]
                idx = idx + 1
            time.sleep(self.shutdown_poll_interval)
        for worker in self.workers:
            try:
                worker.conn.rfile._sock.shutdown(_socket.SHUT_RDWR)
            except (socket.error, AttributeError):
                # Throws attribute error if the file was closed
                # before we reach this instruction
                pass


And at the bottom of CherryPyWSGIServer.stop

     # Must shut down threads here so the code that calls
        # this method can know when all threads are stopped.
        for worker in self._workerThreads:
            self.requests.put(_SHUTDOWNREQUEST)

        # Start socket shutdown monitor
        shutdown_monitor = SocketShutdownMonitor(self._workerThreads)
        shutdown_monitor.start()

        # Don't join currentThread (when stop is called inside a
request).
        current = threading.currentThread()
        while self._workerThreads:
            worker = self._workerThreads.pop()
            if worker is not current and worker.isAlive:
                try:
                    worker.join()
                except (AssertionError, KeyboardInterrupt):
                    pass

        shutdown_monitor.join()


Still need the self.conn assignment on the WorkerThread


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "cherrypy-devel" group.
To post to this group, send email to [hidden email]
To unsubscribe from this group, send email to [hidden email]
For more options, visit this group at http://groups.google.com/group/cherrypy-devel
-~----------~----~----~----~------~----~------~--~---