BIND 10 #2398: msgq crash

[BIND 10 Development]

#2398: msgq crash
-------------------------------------+-------------------------------------
                   Reporter:  jreed  |                 Owner:  jelte
                       Type:         |                Status:  reviewing
  defect                             |             Milestone:
                   Priority:         |  Sprint-20121204
  medium                             |            Resolution:
                  Component:  msgq   |             Sensitive:  0
                   Keywords:         |           Sub-Project:  Core
            Defect Severity:  High   |  Estimated Difficulty:  4
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jelte


Comment:

 Hello

 Replying to [comment:5 jelte]:
 > It does *not* handle and recover any other exceptions (except the socket
 errors that were already handled). I don't think we should catchall here,
 as depending on the exception we should handle it differently (and perhaps
 simply crash, if it is a syntax error for example). Also, I kept it to
 send() calls only. If we want we can do something similar for recv()
 (which should now be a lot easier, as I think the tests could be extended
 to test that too).

 I don't know. For one, I think syntax errors are not descendants of
 Exceptions.
 Even when leaving these out, looking at `man send`, I think there are more
 interesting cases there:
  * `ECONNRESET` (it would probably be similar to `EPIPE`).
  * `ENOBUFS` (the other side probably does not read, or something, so the
 same).
  * `EINTR` (just retry like with `EAGAIN`).

 Also, there are few points to the code:
  * What happens if the `kill_socket` is called (because of `EPIPE`), but
 the
    socket has some data to read and has the read even queued?
  * I think there's +-3 error in this comment:
 {{{
         raise_on_send: integer. If send_exception is not None, it will be
                                 raised on this byte (i.e. 1 = on the first
                                 call to send(), 1 = on the 4th call to
 send)
                                 Note: if 0, send_exception will not be
 raised.
 }}}
  * I don't like timeouts in testing code much. These are probably
 reasonable, because they time out only in case something in the test fail.
 But is 0.2 enough even on slow systems?
 {{{#!python
         # Give it a chance to stop, if it doesn't, no problem, it'll
         # die when the program does
         msgq_thread.join(0.2)
 }}}
  * Also, the code directly below, is it safe? In case the timeout happens,
 the
    thread could still be running. And it could actually be modifying some
 of
    the variables that are being examined. I know this is mostly a
 theoretical
    concern and I know that if it was in the unsafe case, the test would be
    failing anyway, but maybe it would be better to assert first the thread
 is
    not alive.
 {{{#!python
         # Check the exception from the thread, if any
         self.assertEqual(expect_send_exception,
 msgq_thread.caught_exception)
 }}}

-- 
Ticket URL: <http://bind10.isc.org/ticket/2398#comment:7>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development