BIND 10 #2916: define asiolink::IOSocketLocal

BIND 10 Development do-not-reply at isc.org
Wed May 15 12:47:22 UTC 2013


#2916: define asiolink::IOSocketLocal
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  jinmei
            Priority:  medium        |                       Status:
           Component:  data source   |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130528
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  4             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  shared memory data source
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 I wasn't able to run complete tests, due to other problems in master. But
 I believe it doesn't matter, because the problems are in `bin/` and `lib/`
 tests are run sooner.

 The code itself looks good, but I see few problems in the tests.

 You set the `alarm` only sometimes. That's OK. But then, it should be
 reset with `alarm(0)` in the test tear down, so the alarm doesn't fire for
 some completely unrelated test later on, because that one takes too long.

 Also, I know you set the original handler in the test destructor, so it
 doesn't matter, but I still believe it would be cleaner to reset the
 `g_io_service` to NULL as well, because afterwards the variable points to
 something invalid.

 I don't think this will prevent the possible deadlock here:
 {{{#!c++
     alarm(IO_TIMEOUT);
     EXPECT_EQ(data_len, write(sock_pair_[1].get(), &expected_data[0],
                               data_len));
 }}}

 In the theoretical case when write would block with too much data to write
 (and nothing would read on the other side). Then the alarm happens,
 interrupting the write (so it generates failure in the `EXPECT_EQ`), and
 calling `ioservice.stop()`. However, the service is started only *after*
 and it will wait forever for the data that'll never arrive.

 May I suggest leaving the abort to the default (killing the application),
 set it at each start of the test and abort on completion?

 What is the purpose here?
 {{{#!c++
     aux_sockpair[0].set(socks[0]);
     aux_sockpair[1].set(socks[1]);
     LocalSocket aux_sock(io_service_, aux_sockpair[0].release());
 }}}

 If set never throws (it seemed to never throw), then there may be no
 exception between the set and release. If it can, then we can lose
 `socks[1]`.

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


More information about the bind10-tickets mailing list