Defer control channel message invalidation

The conn_shutdown() function is called whenever a control channel
connection is supposed to be closed, e.g. after a response to the client
is sent or when named is being shut down.  That function calls
isccc_ccmsg_invalidate(), which resets the magic number in the structure
holding the messages exchanged over a given control channel connection
(isccc_ccmsg_t).  The expectation here is that all operations related to
the given control channel connection will have been completed by the
time the connection needs to be shut down.

However, if named shutdown is initiated while a control channel message
is still in flight, some netmgr callbacks might still be pending when
conn_shutdown() is called and isccc_ccmsg_t invalidated.  This causes
the REQUIRE assertion checking the magic number in ccmsg_senddone() to
fail when the latter function is eventually called, resulting in a
crash.

Fix by splitting up isccc_ccmsg_invalidate() into two separate
functions:

  - isccc_ccmsg_disconnect(), which initiates TCP connection shutdown,
  - isccc_ccmsg_invalidate(), which cleans up magic number and buffer,

and then:

  - replacing all existing uses of isccc_ccmsg_invalidate() with calls
    to isccc_ccmsg_disconnect(),

  - only calling isccc_ccmsg_invalidate() when all netmgr callbacks are
    guaranteed to have been run.

Adjust function comments accordingly.

diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c
index ae69d22ee050f08c5fca4a6aa77f6567216b2464..46b7ab89280933cc9ebf616b9635fef0e06eacc4 100644 (file)
--- a/bin/named/controlconf.c
+++ b/bin/named/controlconf.c
@@ -419,10 +419,10 @@ conn_shutdown(controlconnection_t *conn) {
        conn->shuttingdown = true;
 
        /*
-        * Calling invalidate on ccmsg will shutdown the TCP connection, thus
-        * we are making sure that no read callback will be called ever again.
+        * Close the TCP connection to make sure that no read callback will be
+        * called for it ever again.
         */
-       isccc_ccmsg_invalidate(&conn->ccmsg);
+       isccc_ccmsg_disconnect(&conn->ccmsg);
 
        /* Detach the reading reference */
        controlconnection_detach(&conn);
@@ -575,6 +575,8 @@ conn_free(controlconnection_t *conn) {
 
        controllistener_t *listener = conn->listener;
 
+       isccc_ccmsg_invalidate(&conn->ccmsg);
+
        conn_cleanup(conn);
 
        if (conn->buffer != NULL) {

diff --git a/bin/rndc/rndc.c b/bin/rndc/rndc.c
index e129fe84348760dc86da86f9a0643ff31e09f096..645cef5ed01fe10f9c48c28556fa219f696a92f3 100644 (file)
--- a/bin/rndc/rndc.c
+++ b/bin/rndc/rndc.c
@@ -348,7 +348,7 @@ rndc_recvdone(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 
        isccc_sexpr_free(&response);
 
-       isccc_ccmsg_invalidate(ccmsg);
+       isccc_ccmsg_disconnect(ccmsg);
        isc_loopmgr_shutdown(loopmgr);
 }
 
@@ -1003,6 +1003,8 @@ main(int argc, char **argv) {
 
        isc_loopmgr_run(loopmgr);
 
+       isccc_ccmsg_invalidate(&rndc_ccmsg);
+
        isc_log_destroy(&log);
        isc_log_setcontext(NULL);
 

diff --git a/lib/isccc/ccmsg.c b/lib/isccc/ccmsg.c
index f52b0391527cce038dbebac886b80e545e3235cd..80d622da792a529870610988a190f4a8692d4bee 100644 (file)
--- a/lib/isccc/ccmsg.c
+++ b/lib/isccc/ccmsg.c
@@ -179,6 +179,16 @@ isccc_ccmsg_sendmessage(isccc_ccmsg_t *ccmsg, isc_region_t *region,
        isc_nm_send(ccmsg->handle, region, ccmsg_senddone, ccmsg);
 }
 
+void
+isccc_ccmsg_disconnect(isccc_ccmsg_t *ccmsg) {
+       REQUIRE(VALID_CCMSG(ccmsg));
+
+       if (ccmsg->handle != NULL) {
+               isc_nmhandle_close(ccmsg->handle);
+               isc_nmhandle_detach(&ccmsg->handle);
+       }
+}
+
 void
 isccc_ccmsg_invalidate(isccc_ccmsg_t *ccmsg) {
        REQUIRE(VALID_CCMSG(ccmsg));
@@ -188,10 +198,6 @@ isccc_ccmsg_invalidate(isccc_ccmsg_t *ccmsg) {
        if (ccmsg->buffer != NULL) {
                isc_buffer_free(&ccmsg->buffer);
        }
-       if (ccmsg->handle != NULL) {
-               isc_nmhandle_close(ccmsg->handle);
-               isc_nmhandle_detach(&ccmsg->handle);
-       }
 }
 
 void

diff --git a/lib/isccc/include/isccc/ccmsg.h b/lib/isccc/include/isccc/ccmsg.h
index 8404252a56f2e4c6ec5fcfb3da2dbf83c0173bf8..7a20a7c2e1a7ad7483d560c318fe99e4873ac500 100644 (file)
--- a/lib/isccc/include/isccc/ccmsg.h
+++ b/lib/isccc/include/isccc/ccmsg.h
@@ -120,18 +120,25 @@ isccc_ccmsg_sendmessage(isccc_ccmsg_t *ccmsg, isc_region_t *region,
  */
 
 void
-isccc_ccmsg_invalidate(isccc_ccmsg_t *ccmsg);
+isccc_ccmsg_disconnect(isccc_ccmsg_t *ccmsg);
 /*%
- * Clean up all allocated state, and invalidate the structure.
+ * Disconnect from the connected netmgr handle associated with a command
+ * channel message.
  *
  * Requires:
  *
- *\li  "ccmsg" be valid.
+ *\li  "ccmsg" to be valid.
+ */
+
+void
+isccc_ccmsg_invalidate(isccc_ccmsg_t *ccmsg);
+/*%
+ * Clean up the magic number and the dynamic buffer associated with a command
+ * channel message.
  *
- * Ensures:
+ * Requires:
  *
- *\li  "ccmsg" is invalidated and disassociated with all memory contexts,
- *     sockets, etc.
+ *\li  "ccmsg" to be valid.
  */
 
 void