ThreadWithException question

[Vincent Ladeuil]

>>>>> John Arbash Meinel <john at arbash-meinel.com> writes:

    > Hey Vincent-
    > I was looking over the code, and I saw something I thought was kind of
    > odd. Specifically this interaction:

    > class ThreadWithException
    > ...
    >     def run(self):
    >         """Overrides Thread.run to capture any exception."""
    >         self.ready.clear()
    >         try:
    >             try:
    >                 super(ThreadWithException, self).run()
    >             except:
    >                 self.exception = sys.exc_info()
    >         finally:
    >             # Make sure the calling thread is released
    >             self.ready.set()

    > vs

    > class TestingTCPServerInAThread(...):
    > ...
    >     def start_server(self):
    >         self.server = self.create_server()
    >         self._server_thread = ThreadWithException(
    >             event=self.server.started,
    >             target=self.run_server)
    >         self._server_thread.start()
    >         # Wait for the server thread to start (i.e release the lock)
    >         self.server.started.wait()


    > Now, for starters the term 'server' appears to mean 4-5 different
    > things, which gets a bit confusing.

<shudder>, I know.

And the modules we are using doesn't help. Let me try to summarize a
bit:

- the test cases interacts with an object that is a proxy to a test
  server. Since that's the code we're dealing with most of the time, we
  call this object a server,
- the real test server which is listening to the socket and handling
  the connections is, well, really a server,
- depending on the server implementation, each received connection will
  start a new thread or fork or run in the same thread as the
  server. A object is used there to handle a connection, but depending
  on the module we use, it's sometimes called a request, a connection or
  even a server. Since that's generally the object that implements the
  protocol (ftp, http, etc), people tend to call it a server. 

    > However the key bit is to notice that 'start_server' is creating a
    > ThreadWithException, starting it, and then waiting on the event to
    > be set before continuing.

    > However, if I read that correctly, ThreadWithException doesn't set
    > its 'ready' flag until the thread has *finished* its run() method
    > (in other words the extra thread is complete and is returning).

Yup, but you miss the whole context there, the daughter class will also
set this event when the server is really ready to serve.

    > Now I see this following:

    > ...
    >         # Get the real address, especially the port
    >         self.host, self.port = self.server.server_address
    >         self._server_thread.name = self.server.server_address
    >         if debug_threads():
    >             sys.stderr.write('Server thread %s started\n'
    >                              % (self._server_thread.name,))
    >         # If an exception occured during the server start, it will get
    > raised,
    >         # otherwise, the server is blocked on its accept() call.
    >         self._server_thread.pending_exception()
    >         # From now on, we'll use a different event to ensure the server
    > can set
    >         # its exception
    >         self._server_thread.set_ready_event(self.server.stopped)

    > ^- It seems *very* strange to be setting the "I'm ready" event to "I'm
    > stopping".

    > In fact, I have the feeling that the code you really want is:

    >         self._server_thread = ThreadWithException(
    >             event=self.server.started,
    >             target=self.run_server)

    > to actually be:
    >         self._server_thread = ThreadWithException(
    >             event=self.server.stopped,
    >             target=self.run_server)

    > I suppose maybe the logic is:
    >   self.server should be setting the 'self.server.started' flag when it
    >   is ready to communicate. However, if it fails to actually startup
    >   properly, we don't want to be waiting forever. So have the Thread's
    >   "I'm exiting" statement set the Event.

    > If this is true, then I suggest we document it a bit better, and not
    > call it 'set_ready_event' since that is misleading. You had it by a
    > different name earlier, but I didn't actually understand what was going
    > on. It really isn't the "ready" event, but actually the
    > "thread_has_stopped" event. (As it is set when the thread is finishing
    > its 'run()' call.)

Again, the thread is driven by the main thread which ask it to do
something and will wait for an event to be set. When the said
'something' is done, the event should be set and the main thread can
continue.

ThreadWithException needs to know which event is currently used to
synchronize so that it can set it if an exception occurs. So 'ready' is
correct, 'done' would be too but 'thread_has_stopped' is definitely
wrong. When the server is ready to accept connections having
'thread_has_stopped' set to True will be non-sensical.

    def set_ready_event(self, event):
        """Set the ``ready`` event used to synchronize exception catching.

        When the thread uses an event to synchronize itself with another thread
        (setting it when the other thread can wake up from a ``wait`` call),
        the event must be set after catching an exception or the other thread
        will hang.

        Some threads require multiple events and should set the relevant one
        when appropriate.
        """
        self.ready = event

Can you suggest a better wording ?

      Vincent