diff --git a/bin/tests/omapi_test.c b/bin/tests/omapi_test.c index 3c77ab461e..5779f8c8d9 100644 --- a/bin/tests/omapi_test.c +++ b/bin/tests/omapi_test.c @@ -430,6 +430,27 @@ do_connect(const char *host, int port) { == ISC_R_SUCCESS); ENSURE(client->waitresult == ISC_R_SUCCESS); + + /* + * Close the connection and wait to be disconnected. + * XXXDCL This problem has been biting my butt for two days + * straight! I am totally zoning on how to best accomplish + * making disconnection be either sync or async, and some + * internal thread race conditions i am having in the omapi library. + * grr grr grr grr GRRRRR + * at the moment things work ok enough by requiring that + * the connection be waited before calling omapi_connection_disconnect + * and sleeping a moment. clearly this is BOGUS + */ + sleep(1); + omapi_connection_disconnect(connection, ISC_FALSE); + + ENSURE(client->waitresult == ISC_R_SUCCESS); + + /* + * Free the protocol manager. + */ + omapi_object_dereference(&manager, "do_connect"); } static void @@ -529,6 +550,8 @@ main (int argc, char **argv) { exit (1); } + omapi_shutdown(); + if (show_final_mem) isc_mem_stats(mctx, stderr); diff --git a/lib/omapi/connection.c b/lib/omapi/connection.c index e8e667870c..79b74c8662 100644 --- a/lib/omapi/connection.c +++ b/lib/omapi/connection.c @@ -15,7 +15,7 @@ * SOFTWARE. */ -/* $Id: connection.c,v 1.7 2000/01/13 06:13:21 tale Exp $ */ +/* $Id: connection.c,v 1.8 2000/01/14 23:10:01 tale Exp $ */ /* Principal Author: Ted Lemon */ @@ -73,33 +73,20 @@ get_address(const char *hostname, in_port_t port, isc_sockaddr_t *sockaddr) { return (ISC_R_SUCCESS); } +/* + * Called when there are no more events are pending on the socket. + * It can be detached and data for the connection object freed. + */ static void -abandon_connection(omapi_connection_object_t *connection, - isc_event_t *event, isc_result_t result) -{ +free_connection(omapi_connection_object_t *connection) { isc_buffer_t *buffer; - if (event != NULL) - isc_event_free(&event); - - if (connection->events_pending > 0) { - /* - * The only time CANCELED results should be generated is - * because this function already called isc_socket_cancel. - * If this isn't a CANCELED result, then the isc_socket_cancel - * needs to be done. - */ - if (result != ISC_R_CANCELED) - isc_socket_cancel(connection->socket, NULL, - ISC_SOCKCANCEL_ALL); - - /* - * Technically not yet, but the end result is the same. - */ - connection->state = omapi_connection_unconnected; - - return; - } + /* + * The mutex is locked when this routine is called. Unlock + * it so that the isc_condition_signal below will allow + * omapi_connection_wait to be able to acquire the lock. + */ + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == ISC_R_SUCCESS); while ((buffer = ISC_LIST_HEAD(connection->input_buffers)) != NULL) { ISC_LIST_UNLINK(connection->input_buffers, buffer, link); @@ -111,12 +98,97 @@ abandon_connection(omapi_connection_object_t *connection, isc_buffer_free(&buffer); } - isc_task_destroy(&connection->task); - isc_socket_detach(&connection->socket); + if (connection->task != NULL) + isc_task_destroy(&connection->task); - OBJECT_DEREF(&connection, "abandon_connection"); + if (connection->socket != NULL) + isc_socket_detach(&connection->socket); - return; + /* + * It is possible the connection was abandoned while a message + * was being assembled. That thread needs to be unblocked. + * Make sure that when it is awakened, it exits the loop by + * setting messages_expected to 0. + */ + if (connection->is_client) { + connection->messages_expected = 0; + + REQUIRE(isc_condition_signal(&connection->waiter) == + ISC_R_SUCCESS); + RUNTIME_CHECK(isc_condition_destroy(&connection->waiter) == + ISC_R_SUCCESS); + } + + /* XXXDCL ok, but what happens when omapi_connection_wait + * awakens and then tries to unlock the mutex, which is possibly + * being destroyed right here. i don't think relocking is + * a guarantee that isc_condition_wait exited and the mutex was + * unlocked in omapi_connection_wait. still, it's worth a shot. + */ + RUNTIME_CHECK(isc_mutex_lock(&connection->mutex) == ISC_R_SUCCESS); + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == ISC_R_SUCCESS); + RUNTIME_CHECK(isc_mutex_destroy(&connection->mutex) == ISC_R_SUCCESS); + + /* + * Disconnect from I/O object, if any. + */ + if (connection->outer != NULL) + OBJECT_DEREF(&connection->outer, + "omapi_connection_disconnect"); + + /* + * If whatever created us registered a signal handler, send it + * a disconnect signal. + */ + omapi_signal((omapi_object_t *)connection, "disconnect", connection); + + /* + * Finally, free the object itself. + */ + OBJECT_DEREF(&connection, "free_connection"); +} + +static void +end_connection(omapi_connection_object_t *connection, isc_event_t *event, + isc_result_t result) +{ + if (event != NULL) + isc_event_free(&event); + + /* + * XXXDCL would be nice to send the result as an + * omapi_signal(object, "status", result) but i don't + * think this can be done with the connection as the object. + */ + + /* + * Lock the connection's mutex to examine connection->events_pending. + */ + RUNTIME_CHECK(isc_mutex_lock(&connection->mutex) == ISC_R_SUCCESS); + + fprintf(stderr, "END_CONNECTION, %d events_pending\n", + connection->events_pending); + + if (connection->events_pending == 0) { + free_connection(connection); + return; + } + + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == + ISC_R_SUCCESS); + + /* + * There are events pending. Cancel them, and each will generate + * a call with ISC_R_CANCELED to this routine until finally + * events_pending is 0 and the connection is freed. The + * only time ISC_R_CANCELED should be generated is after this + * function already called isc_socket_cancel, because it is + * the only place in the omapi library that isc_socket_cancel is used. + */ + + if (result != ISC_R_CANCELED) + isc_socket_cancel(connection->socket, NULL, + ISC_SOCKCANCEL_ALL); } /* @@ -134,9 +206,24 @@ connect_done(isc_task_t *task, isc_event_t *event) { connectevent = (isc_socket_connev_t *)event; connection = event->arg; + fprintf(stderr, "CONNECT_DONE\n"); + ENSURE(socket == connection->socket && task == connection->task); - connection->events_pending--; + RUNTIME_CHECK(isc_mutex_lock(&connection->mutex) == ISC_R_SUCCESS); + /* + * XXXDCL I may have made an unwarranted assumption about + * events_pending becoming 0 on the client when disconnecting + * only in recv_done. I'm concerned that there might be some + * sort of logic window, however small, where that isn't true. + */ + ENSURE(connection->events_pending > 0); + if (--connection->events_pending == 0 && connection->is_client && + connection->state == omapi_connection_disconnecting) + FATAL_ERROR(__FILE__, __LINE__, + "events_pending == 0 in connect_done while " + "disconnecting, this should not happen!"); + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == ISC_R_SUCCESS); /* * XXXDCL For some reason, a "connection refused" error is not @@ -144,30 +231,28 @@ connect_done(isc_task_t *task, isc_event_t *event) { * how that error is indicated. */ - if (connectevent->result != ISC_R_SUCCESS) { - abandon_connection(connection, event, connectevent->result); - return; - } + if (connectevent->result != ISC_R_SUCCESS) + goto abandon; result = isc_socket_getpeername(connection->socket, &connection->remote_addr); - if (result != ISC_R_SUCCESS) { - abandon_connection(connection, event, connectevent->result); - return; - } + if (result != ISC_R_SUCCESS) + goto abandon; result = isc_socket_getsockname(connection->socket, &connection->local_addr); - if (result != ISC_R_SUCCESS) { - abandon_connection(connection, event, connectevent->result); - return; - } + if (result != ISC_R_SUCCESS) + goto abandon; connection->state = omapi_connection_connected; isc_event_free(&event); return; + +abandon: + end_connection(connection, event, connectevent->result); + return; } /* @@ -186,9 +271,14 @@ recv_done(isc_task_t *task, isc_event_t *event) { socketevent = (isc_socketevent_t *)event; connection = event->arg; + fprintf(stderr, "RECV_DONE, %d bytes\n", socketevent->n); + ENSURE(socket == connection->socket && task == connection->task); + RUNTIME_CHECK(isc_mutex_lock(&connection->mutex) == ISC_R_SUCCESS); + ENSURE(connection->events_pending > 0); connection->events_pending--; + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == ISC_R_SUCCESS); /* * Restore the input buffers to the connection object. @@ -198,10 +288,8 @@ recv_done(isc_task_t *task, isc_event_t *event) { buffer = ISC_LIST_NEXT(buffer, link)) ISC_LIST_APPEND(connection->input_buffers, buffer, link); - if (socketevent->result != ISC_R_SUCCESS) { - abandon_connection(connection, event, socketevent->result); - return; - } + if (socketevent->result != ISC_R_SUCCESS) + goto abandon; connection->in_bytes += socketevent->n; @@ -209,8 +297,10 @@ recv_done(isc_task_t *task, isc_event_t *event) { while (connection->bytes_needed <= connection->in_bytes && connection->bytes_needed > 0) - omapi_signal((omapi_object_t *)connection, "ready", - connection); + if (omapi_signal((omapi_object_t *)connection, "ready", + connection) != + ISC_R_SUCCESS) + goto abandon; /* * Queue up another recv request. If the bufferlist is empty, @@ -221,8 +311,36 @@ recv_done(isc_task_t *task, isc_event_t *event) { if (! ISC_LIST_EMPTY(connection->input_buffers)) omapi_connection_require((omapi_object_t *)connection, 0); - isc_event_free(&event); + /* + * See if that was the last event the client was expecting, so + * that the connection can be freed. + * XXXDCL I don't *think* this has to be done in the + * send_done or connect_done handlers, because a normal + * termination will only happen + */ + if (connection->is_client) { + RUNTIME_CHECK(isc_mutex_lock(&connection->mutex) == + ISC_R_SUCCESS); + if (connection->events_pending == 0 && + connection->state == omapi_connection_disconnecting) { + ENSURE(connection->messages_expected == 1); + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == + ISC_R_SUCCESS); + + end_connection(connection, event, ISC_R_SUCCESS); + return; + } + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == + ISC_R_SUCCESS); + } + + isc_event_free(&event); + return; + +abandon: + end_connection(connection, event, socketevent->result); + return; } /* @@ -240,6 +358,8 @@ send_done(isc_task_t *task, isc_event_t *event) { socketevent = (isc_socketevent_t *)event; connection = event->arg; + fprintf(stderr, "SEND_DONE, %d bytes\n", socketevent->n); + ENSURE(socket == connection->socket && task == connection->task); /* @@ -250,13 +370,26 @@ send_done(isc_task_t *task, isc_event_t *event) { socketevent->n == isc_bufferlist_usedcount(&socketevent->bufferlist)); - connection->events_pending--; + RUNTIME_CHECK(isc_mutex_lock(&connection->mutex) == ISC_R_SUCCESS); + /* + * XXXDCL I may have made an unwarranted assumption about + * events_pending becoming 0 on the client when disconnecting + * only in recv_done. I'm concerned that there might be some + * sort of logic window, however small, where that isn't true. + */ + ENSURE(connection->events_pending > 0); + if (--connection->events_pending == 0 && connection->is_client && + connection->state == omapi_connection_disconnecting) + FATAL_ERROR(__FILE__, __LINE__, + "events_pending == 0 in send_done while " + "disconnecting, this should not happen!"); + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == ISC_R_SUCCESS); /* * Restore the head of bufferlist into the connection object, resetting * it to have zero used space, and free the remaining buffers. * This is done before the test of the socketevent's result so that - * abandon_connection() can free the buffer, if it is called below. + * end_connection() can free the buffer, if it is called below. */ buffer = ISC_LIST_HEAD(socketevent->bufferlist); ISC_LIST_APPEND(connection->output_buffers, buffer, link); @@ -267,15 +400,16 @@ send_done(isc_task_t *task, isc_event_t *event) { isc_buffer_free(&buffer); } - if (socketevent->result != ISC_R_SUCCESS) { - abandon_connection(connection, event, socketevent->result); - return; - } + if (socketevent->result != ISC_R_SUCCESS) + goto abandon; connection->out_bytes -= socketevent->n; isc_event_free(&event); + return; +abandon: + end_connection(connection, event, socketevent->result); return; } @@ -284,14 +418,20 @@ connection_send(omapi_connection_object_t *connection) { REQUIRE(connection != NULL && connection->type == omapi_type_connection); + REQUIRE(connection->state == omapi_connection_connected); + if (connection->out_bytes > 0) { ENSURE(!ISC_LIST_EMPTY(connection->output_buffers)); + RUNTIME_CHECK(isc_mutex_lock(&connection->mutex) == + ISC_R_SUCCESS); + connection->events_pending++; + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == + ISC_R_SUCCESS); + isc_socket_sendv(connection->socket, &connection->output_buffers, connection->task, send_done, connection); - - connection->events_pending++; } } @@ -322,30 +462,21 @@ omapi_connection_toserver(omapi_object_t *protocol, const char *server_name, result = isc_buffer_allocate(omapi_mctx, &ibuffer, OMAPI_BUFFER_SIZE, ISC_BUFFERTYPE_BINARY); - if (result != ISC_R_SUCCESS) { - isc_task_destroy(&task); - return (result); - } + if (result != ISC_R_SUCCESS) + goto free_task; result = isc_buffer_allocate(omapi_mctx, &obuffer, OMAPI_BUFFER_SIZE, ISC_BUFFERTYPE_BINARY); - if (result != ISC_R_SUCCESS) { - isc_buffer_free(&ibuffer); - isc_task_destroy(&task); - return (result); - } + if (result != ISC_R_SUCCESS) + goto free_ibuffer; /* * Create a new connection object. */ result = omapi_object_new((omapi_object_t **)&connection, omapi_type_connection, sizeof(*connection)); - if (result != ISC_R_SUCCESS) { - isc_buffer_free(&obuffer); - isc_buffer_free(&ibuffer); - isc_task_destroy(&task); - return (result); - } + if (result != ISC_R_SUCCESS) + goto free_obuffer; connection->is_client = ISC_TRUE; @@ -356,13 +487,9 @@ omapi_connection_toserver(omapi_object_t *protocol, const char *server_name, ISC_LIST_INIT(connection->output_buffers); ISC_LIST_APPEND(connection->output_buffers, obuffer, link); - result = isc_mutex_init(&connection->mutex); - if (result != ISC_R_SUCCESS) - return (result); - - result = isc_condition_init(&connection->waiter); - if (result != ISC_R_SUCCESS) - return (result); + RUNTIME_CHECK(isc_mutex_init(&connection->mutex) == ISC_R_SUCCESS); + RUNTIME_CHECK(isc_condition_init(&connection->waiter) == + ISC_R_SUCCESS); /* * An introductory message is expected from the server. @@ -384,17 +511,8 @@ omapi_connection_toserver(omapi_object_t *protocol, const char *server_name, */ result = isc_socket_create(omapi_socketmgr, isc_sockaddr_pf(&sockaddr), isc_sockettype_tcp, &connection->socket); - if (result != ISC_R_SUCCESS) { - /* XXXDCL this call and later will not free the connection obj - * because it has two refcnts, one for existing plus one - * for the tie to h->outer. This does not seem right to me. - */ - OBJECT_DEREF(&connection, "omapi_connection_toserver"); - isc_buffer_free(&obuffer); - isc_buffer_free(&ibuffer); - isc_task_destroy(&task); - return (result); - } + if (result != ISC_R_SUCCESS) + goto free_object; #if 0 /* @@ -409,14 +527,30 @@ omapi_connection_toserver(omapi_object_t *protocol, const char *server_name, } #endif + RUNTIME_CHECK(isc_mutex_lock(&connection->mutex) == ISC_R_SUCCESS); + connection->events_pending++; + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == ISC_R_SUCCESS); + result = isc_socket_connect(connection->socket, &sockaddr, task, connect_done, connection); if (result != ISC_R_SUCCESS) { - abandon_connection(connection, NULL, result); + end_connection(connection, NULL, result); return (result); } - connection->events_pending++; + return (ISC_R_SUCCESS); + +free_object: + OBJECT_DEREF(&connection, "omapi_connection_toserver"); + OBJECT_DEREF(&protocol->outer, "omapi_connection_toserver"); + return (result); + +free_obuffer: + isc_buffer_free(&obuffer); +free_ibuffer: + isc_buffer_free(&ibuffer); +free_task: + isc_task_destroy(&task); return (result); } @@ -540,8 +674,31 @@ omapi_connection_copyout(unsigned char *dst, omapi_object_t *generic, * Disconnect a connection object from the remote end. If force is true, * close the connection immediately. Otherwise, shut down the receiving end * but allow any unsent data to be sent before actually closing the socket. + * + * This routine is called in the following situations: + * + * The client wants to exit normally after all its transactions are + * processed. Closing the connection causes an ISC_R_EOF event result + * to be given to the server's recv_done, which then causes the + * server's recv_done to close its side of the connection. + * + * The client got some sort of error it could not handle gracefully, so + * it wants to just tear down the connection. This can be caused either + * internally in the omapi library, or by the calling program. + * + * The server is dropping the connection. This is always asynchronous; + * the server will never block waiting for a connection to be completed + * because it never initiates a "normal" close of the connection. + * (Receipt of ISC_R_EOF is always treated as though it were an error, + * no matter what the client had been intending; it's the nature of + * the protocol.) + * + * The client might or might not want to block on the disconnection. + * Also, if the error is being thrown from the library, the client + * might *already* be waiting on (or intending to wait on) whatever messages + * it has already sent, so it needs to be awakened. That will be handled + * by free_connection after all of the cancelled events are processed. */ - void omapi_connection_disconnect(omapi_object_t *generic, isc_boolean_t force) { omapi_connection_object_t *connection; @@ -552,47 +709,60 @@ omapi_connection_disconnect(omapi_object_t *generic, isc_boolean_t force) { REQUIRE(connection->type == omapi_type_connection); + /* + * Only the client can request an unforced disconnection. The server's + * disconnection will always happen when the client goes away. + * XXXDCL ... hmm, can timeouts of the client on the server be handled? + */ + REQUIRE(force || connection->is_client); + + /* + * XXXDCL this has to be fixed up when isc_socket_shutdown is + * available, because then the shutdown can be done asynchronously. + * It is currently done synchronously. + */ + if (! force) { /* - * If we're already disconnecting, we don't have to do - * anything. + * Client wants a clean disconnect. + * + * Increment the count of messages expected. Even though + * no message is really expected, this will keep + * omapi_connection_wait from exiting until free_connection() + * signals it. */ - if (connection->state == omapi_connection_disconnecting) - return; + RUNTIME_CHECK(isc_mutex_lock(&connection->mutex) == + ISC_R_SUCCESS); + + connection->state = omapi_connection_disconnecting; + connection->messages_expected++; /* - * Try to shut down the socket - this sends a FIN to the - * remote end, so that it won't send us any more data. If - * the shutdown succeeds, and we still have bytes left to - * write, defer closing the socket until that's done. + * If there are no other messages expected for the socket, + * the end_connection can be done right now. Otherwise, + * when recv_done gets the last output from the server, + * then it will then end the connection. */ - if (connection->out_bytes > 0) { -#if 0 /*XXXDCL*/ - isc_socket_shutdown(connection->socket, - ISC_SOCKSHUT_RECV); -#else - isc_socket_cancel(connection->socket, NULL, - ISC_SOCKCANCEL_RECV); -#endif - connection->state = omapi_connection_disconnecting; + if (connection->messages_expected > 1) { + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == + ISC_R_SUCCESS); return; } + /* + * ... else fall through. + */ + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == + ISC_R_SUCCESS); } - - isc_task_shutdown(connection->task); - connection->state = omapi_connection_closed; - + /* - * Disconnect from I/O object, if any. + * XXXDCL + * This might be improved if the 'force' argument to this function + * were instead an isc_reault_t argument. Then omapi_signal could send + * a "status" back up to a signal handler that could set a waitresult. */ - if (connection->outer != NULL) - OBJECT_DEREF(&connection->outer, "omapi_connection_disconnect"); - - /* - * If whatever created us registered a signal handler, send it - * a disconnect signal. - */ - omapi_signal(generic, "disconnect", generic); + end_connection(connection, NULL, + force ? ISC_R_UNEXPECTED : ISC_R_SUCCESS); } /* @@ -607,6 +777,25 @@ omapi_connection_require(omapi_object_t *generic, unsigned int bytes) { connection = (omapi_connection_object_t *)generic; + /* + * There is a race condition here that is really, REALLY + * irritating me. Here's the latest attempt at fixing the + * whole dang disconnect problem. It is doomed to fail ... + * but maybe it will do the job for now. + * XXXDCL + */ + RUNTIME_CHECK(isc_mutex_lock(&connection->mutex) == ISC_R_SUCCESS); + if (connection->state == omapi_connection_disconnecting && + connection->messages_expected == 1) { + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == + ISC_R_SUCCESS); + return (OMAPI_R_NOTYET); + } + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == ISC_R_SUCCESS); + + ENSURE(connection->state == omapi_connection_connected || + connection->state == omapi_connection_disconnecting); + connection->bytes_needed += bytes; if (connection->bytes_needed <= connection->in_bytes) @@ -667,15 +856,42 @@ omapi_connection_require(omapi_object_t *generic, unsigned int bytes) { } } + RUNTIME_CHECK(isc_mutex_lock(&connection->mutex) == ISC_R_SUCCESS); /* - * Queue the receive task. - * XXXDCL The "minimum" argument has not been fully thought out. + * Don't queue any more read requests if the connection is + * disconnecting and no more messages are expected. This situation + * can happen when omapi_connection_disconnect is called before + * omapi_protocol_signal_handler has had a chance to go back to + * its omapi_protocol_header_wait condition where it calls + * omapi_connection_require. It is possible the isc_socket_cancel + * was already done. */ - isc_socket_recvv(connection->socket, &connection->input_buffers, - connection->bytes_needed - connection->in_bytes, - connection->task, recv_done, connection); + if (connection->state == omapi_connection_disconnecting && + connection->messages_expected == 0) { + /* + * Test the above comment's claim that this only happens + * when requesting to wait for the header of the next message. + * XXXDCL + * (omapi_protocol_object_t *)p->inner->header_size should + * be what is checked against, but at the present time + * the protocol_object_t exists only in protocol.c + */ + ENSURE(bytes == 24); + } else { + /* + * Queue the receive task. + * XXXDCL The "minimum" arg has not been fully thought out. + */ + connection->events_pending++; + isc_socket_recvv(connection->socket, + &connection->input_buffers, + (connection->bytes_needed - + connection->in_bytes), + connection->task, recv_done, connection); + } - connection->events_pending++; + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == + ISC_R_SUCCESS); return (OMAPI_R_NOTYET); } @@ -694,7 +910,7 @@ omapi_connection_wait(omapi_object_t *object, isc_time_t *timeout) { /* - * 'object' is not really used. + * XXXDCL 'object' is not really used. */ omapi_connection_object_t *connection; isc_result_t result, wait_result; @@ -715,7 +931,8 @@ omapi_connection_wait(omapi_object_t *object, wait_result = ISC_R_SUCCESS; while (connection->messages_expected > 0 && - wait_result == ISC_R_SUCCESS) { + wait_result == ISC_R_SUCCESS) + if (timeout == NULL) wait_result = isc_condition_wait(&connection->waiter, &connection->mutex); @@ -724,13 +941,11 @@ omapi_connection_wait(omapi_object_t *object, isc_condition_waituntil(&connection->waiter, &connection->mutex, timeout); - } - if (wait_result == ISC_R_SUCCESS || wait_result == ISC_R_TIMEDOUT) { - result = isc_mutex_unlock(&connection->mutex); - if (result != ISC_R_SUCCESS) - return (result); - } + RUNTIME_CHECK(wait_result == ISC_R_SUCCESS || + wait_result == ISC_R_TIMEDOUT); + + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == ISC_R_SUCCESS); return (wait_result); } @@ -787,7 +1002,8 @@ isc_result_t omapi_connection_stuffvalues(omapi_object_t *connection, omapi_object_t *id, omapi_object_t *handle) { - REQUIRE(connection != NULL && connection->type == omapi_type_connection); + REQUIRE(connection != NULL && + connection->type == omapi_type_connection); PASS_STUFFVALUES(handle); } diff --git a/lib/omapi/include/omapi/omapip.h b/lib/omapi/include/omapi/omapip.h index e51d76174a..7f82038425 100644 --- a/lib/omapi/include/omapi/omapip.h +++ b/lib/omapi/include/omapi/omapip.h @@ -482,6 +482,9 @@ omapi_message_process(omapi_object_t *message, omapi_object_t *protocol); isc_result_t omapi_init(isc_mem_t *mctx); +void +omapi_shutdown(void); + isc_result_t omapi_object_type_register(omapi_object_type_t **type, const char *name, diff --git a/lib/omapi/listener.c b/lib/omapi/listener.c index 1642042279..2e94f52af7 100644 --- a/lib/omapi/listener.c +++ b/lib/omapi/listener.c @@ -24,6 +24,7 @@ #include #include +#include #include #include @@ -51,6 +52,7 @@ omapi_listener_accept(isc_task_t *task, isc_event_t *event) { * XXXDCL What are the meaningful things the listen/accept function * can do if it fails to process an incoming connection because one * of the functions it calls fails? + * The cleanup options are hurting my head. */ /* @@ -68,7 +70,7 @@ omapi_listener_accept(isc_task_t *task, isc_event_t *event) { * The result is probably ISC_R_UNEXPECTED; what can really be * done about this other than just flunking out of here? */ - return; + goto free_event; /* * The new connection is good to go. Allocate the buffers for it and @@ -76,60 +78,81 @@ omapi_listener_accept(isc_task_t *task, isc_event_t *event) { */ if (isc_task_create(omapi_taskmgr, NULL, 0, &connection_task) != ISC_R_SUCCESS) - return; + goto free_task; ibuffer = NULL; result = isc_buffer_allocate(omapi_mctx, &ibuffer, OMAPI_BUFFER_SIZE, ISC_BUFFERTYPE_BINARY); if (result != ISC_R_SUCCESS) - return; + goto free_ibuffer; obuffer = NULL; result = isc_buffer_allocate(omapi_mctx, &obuffer, OMAPI_BUFFER_SIZE, ISC_BUFFERTYPE_BINARY); if (result != ISC_R_SUCCESS) - return; + goto free_obuffer; /* * Create a new connection object. */ result = omapi_object_new((omapi_object_t **)&connection, omapi_type_connection, sizeof(*connection)); - if (result != ISC_R_SUCCESS) { - /* XXXDCL cleanup */ - isc_buffer_free(&obuffer); - isc_buffer_free(&ibuffer); - return; - } + if (result != ISC_R_SUCCESS) + goto free_obuffer; connection->task = connection_task; connection->state = omapi_connection_connected; connection->socket = incoming->newsocket; connection->is_client = ISC_FALSE; + /* + * No more need for the event, once all the desired data has been + * used from it. + */ + listener = event->arg; + isc_event_free(&event); + ISC_LIST_INIT(connection->input_buffers); ISC_LIST_APPEND(connection->input_buffers, ibuffer, link); ISC_LIST_INIT(connection->output_buffers); ISC_LIST_APPEND(connection->output_buffers, obuffer, link); + RUNTIME_CHECK(isc_mutex_init(&connection->mutex) == ISC_R_SUCCESS); + /* * Notify the listener object that a connection was made. */ - listener = event->arg; result = omapi_signal(listener, "connect", connection); if (result != ISC_R_SUCCESS) - /*XXXDCL then what?!*/ - ; + goto free_object; /* - * Lose our reference to the connection, so it'll be gc'd when it's - * reaped. - * XXXDCL ... um, hmm? this object only has one reference, so it - * is going to be reaped right here! unless omapi_signal added - * a reference ... + * Lose one reference to the connection, so it'll be gc'd when it's + * reaped. The omapi_protocol_listener_signal function added a + * reference when it created a protocol object as connection->inner. */ OBJECT_DEREF(&connection, "omapi_listener_accept"); + return; +free_object: + /* + * Destroy the connection. This will free everything created + * in this function but the event. + */ + OBJECT_DEREF(&connection, "omapi_listener_accept"); + return; + + /* + * Free resources that were being created for the connection object. + */ +free_obuffer: + isc_buffer_free(&obuffer); +free_ibuffer: + isc_buffer_free(&ibuffer); +free_task: + isc_task_destroy(&connection_task); +free_event: + isc_event_free(&event); return; } diff --git a/lib/omapi/message.c b/lib/omapi/message.c index 6c78d549c1..24ab62757d 100644 --- a/lib/omapi/message.c +++ b/lib/omapi/message.c @@ -444,7 +444,8 @@ omapi_message_process(omapi_object_t *mo, omapi_object_t *po) { if (type == NULL) { if (create != 0) { return (omapi_protocol_send_status(po, NULL, - OMAPI_R_INVALIDARG, message->id, + OMAPI_R_INVALIDARG, + message->id, "type required on create")); } goto refresh; @@ -545,7 +546,8 @@ omapi_message_process(omapi_object_t *mo, omapi_object_t *po) { case OMAPI_OP_UPDATE: if (m->object != NULL) { - OBJECT_REF(&object, m->object, "omapi_message_process"); + OBJECT_REF(&object, m->object, + "omapi_message_process"); } else { result = omapi_handle_lookup(&object, message->handle); if (result != ISC_R_SUCCESS) { diff --git a/lib/omapi/object.c b/lib/omapi/object.c index bfa4655876..b8733d571b 100644 --- a/lib/omapi/object.c +++ b/lib/omapi/object.c @@ -15,7 +15,7 @@ * SOFTWARE. */ -/* $Id: object.c,v 1.3 2000/01/06 23:53:00 tale Exp $ */ +/* $Id: object.c,v 1.4 2000/01/14 23:10:03 tale Exp $ */ /* Principal Author: Ted Lemon */ @@ -30,7 +30,8 @@ #include isc_result_t -omapi_object_new(omapi_object_t **object, omapi_object_type_t *type, size_t size) +omapi_object_new(omapi_object_t **object, omapi_object_type_t *type, + size_t size) { omapi_object_t *new; @@ -76,10 +77,17 @@ omapi_object_dereference(omapi_object_t **h, const char *name) { REQUIRE((*h)->refcnt > 0); /* - * See if this object's inner object refers to it, but don't + * See if this object's inner object refers back to it, but don't * count this as a reference if we're being asked to free the * reference from the inner object. */ + /* + * XXXDCL my wording + * Note whether the object being dereference has an inner object, but + * only if the inner object's own outer pointer is not what is + * being dereferenced. + * (XXXDCL when does it happen that way ?) + */ if ((*h)->inner != NULL && (*h)->inner->outer != NULL && h != &((*h)->inner->outer)) inner_reference = 1; @@ -120,6 +128,7 @@ omapi_object_dereference(omapi_object_t **h, const char *name) { * handle table here. */ extra_references = 0; + for (p = (*h)->inner; p != NULL && extra_references == 0; p = p->inner) { @@ -129,6 +138,7 @@ omapi_object_dereference(omapi_object_t **h, const char *name) { if (p->handle != 0) --extra_references; } + for (p = (*h)->outer; p != NULL && extra_references == 0; p = p->outer) { diff --git a/lib/omapi/protocol.c b/lib/omapi/protocol.c index 8852b7c0eb..ba350b622b 100644 --- a/lib/omapi/protocol.c +++ b/lib/omapi/protocol.c @@ -125,7 +125,8 @@ omapi_protocol_disconnect(omapi_object_t *handle, isc_boolean_t force) { connection = (omapi_connection_object_t *)protocol->outer; - ENSURE(connection != NULL && connection->type == omapi_type_connection); + ENSURE(connection != NULL && + connection->type == omapi_type_connection); omapi_connection_disconnect((omapi_object_t *)connection, force); } @@ -209,19 +210,15 @@ omapi_protocol_send_message(omapi_object_t *po, omapi_object_t *id, /* XXXTL Write the ID of the authentication key we're using. */ result = omapi_connection_putuint32(c, 0); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(c, OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; /* * Write the opcode. */ result = omapi_connection_putuint32(c, m->op); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(c, OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; /* * Write the handle. If we've been given an explicit handle, use @@ -233,49 +230,39 @@ omapi_protocol_send_message(omapi_object_t *po, omapi_object_t *id, : (m->object ? m->object->handle : 0))); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(c, OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; /* * Set and write the transaction ID. */ m->id = p->next_xid++; result = omapi_connection_putuint32(c, m->id); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(c, OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; /* * Write the transaction ID of the message to which this is a * response, if there is such a message. */ result = omapi_connection_putuint32(c, om ? om->id : m->rid); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(c, OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; /* * Stuff out the name/value pairs specific to this message. */ result = omapi_stuff_values(c, id, (omapi_object_t *)m); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(c, OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; /* * Write the zero-length name that terminates the list of name/value * pairs specific to the message. */ result = omapi_connection_putuint16(c, 0); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(c, OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; /* * Stuff out all the published name/value pairs in the object that's @@ -283,10 +270,8 @@ omapi_protocol_send_message(omapi_object_t *po, omapi_object_t *id, */ if (m->object != NULL) { result = omapi_stuff_values(c, id, m->object); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(c, OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; } /* @@ -294,10 +279,8 @@ omapi_protocol_send_message(omapi_object_t *po, omapi_object_t *id, * pairs for the associated object. */ result = omapi_connection_putuint16(c, 0); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(c, OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; /* XXXTL Write the authenticator... */ @@ -306,16 +289,13 @@ omapi_protocol_send_message(omapi_object_t *po, omapi_object_t *id, * When the client sends a message, it expects a reply. */ if (connection->is_client) { - result = isc_mutex_lock(&connection->mutex); - if (result != ISC_R_SUCCESS) - goto disconnect; + RUNTIME_CHECK(isc_mutex_lock(&connection->mutex) == + ISC_R_SUCCESS); connection->messages_expected++; - result = isc_mutex_unlock(&connection->mutex); - if (result != ISC_R_SUCCESS) - goto disconnect; - + RUNTIME_CHECK(isc_mutex_unlock(&connection->mutex) == + ISC_R_SUCCESS); } connection_send(connection); @@ -378,15 +358,13 @@ omapi_protocol_signal_handler(omapi_object_t *h, const char *name, va_list ap) * We currently only support the current protocol version. */ if (p->protocol_version != OMAPI_PROTOCOL_VERSION) { - omapi_connection_disconnect(connection, - OMAPI_FORCE_DISCONNECT); - return (OMAPI_R_VERSIONMISMATCH); + result = OMAPI_R_VERSIONMISMATCH; + goto disconnect; } if (p->header_size < sizeof(omapi_protocol_header_t)) { - omapi_connection_disconnect(connection, - OMAPI_FORCE_DISCONNECT); - return (OMAPI_R_PROTOCOLERROR); + result = OMAPI_R_PROTOCOLERROR; + goto disconnect; } /* @@ -395,30 +373,23 @@ omapi_protocol_signal_handler(omapi_object_t *h, const char *name, va_list ap) * XXXDCL duplicated below */ if (c->is_client) { - result = isc_mutex_lock(&c->mutex); - if (result != ISC_R_SUCCESS) - goto disconnect; + RUNTIME_CHECK(isc_mutex_lock(&c->mutex) == + ISC_R_SUCCESS); - /* - * This is an unsigned int but on the server it will - * count below 0 for each incoming message received. - * But that's ok, because the server doesn't support - * omapi_connection_wait. - */ + ENSURE(c->messages_expected > 0); c->messages_expected--; - result = isc_condition_signal(&c->waiter); + RUNTIME_CHECK(isc_condition_signal(&c->waiter) == + ISC_R_SUCCESS); /* * Release the lock. Contrary to what you might think * from some documentation sources, it is necessary * to do this for the waiting thread to unblock. */ - if (result == ISC_R_SUCCESS) - result = isc_mutex_unlock(&c->mutex); + RUNTIME_CHECK(isc_mutex_unlock(&c->mutex) == + ISC_R_SUCCESS); - if (result != ISC_R_SUCCESS) - goto disconnect; } to_header_wait: @@ -442,11 +413,8 @@ omapi_protocol_signal_handler(omapi_object_t *h, const char *name, va_list ap) case omapi_protocol_header_wait: result = omapi_message_new((omapi_object_t **)&p->message, "omapi_protocol_signal_handler"); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(connection, - OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; /* * Swap in the header. @@ -504,11 +472,8 @@ omapi_protocol_signal_handler(omapi_object_t *h, const char *name, va_list ap) case omapi_protocol_name_length_wait: result = omapi_connection_getuint16(connection, &nlen); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(connection, - OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; /* * A zero-length name means that we're done reading name+value @@ -558,11 +523,9 @@ omapi_protocol_signal_handler(omapi_object_t *h, const char *name, va_list ap) */ result = omapi_data_newstring(&p->name, nlen, "omapi_protocol_signal_handler"); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(connection, - OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; + p->state = omapi_protocol_name_wait; if (omapi_connection_require(connection, nlen) != ISC_R_SUCCESS) @@ -575,11 +538,8 @@ omapi_protocol_signal_handler(omapi_object_t *h, const char *name, va_list ap) case omapi_protocol_name_wait: result = omapi_connection_copyout(p->name->value, connection, p->name->len); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(connection, - OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; /* * Wait for a 32-bit length. @@ -606,11 +566,8 @@ omapi_protocol_signal_handler(omapi_object_t *h, const char *name, va_list ap) result = omapi_data_new(&p->value, omapi_datatype_data, vlen, "omapi_protocol_signal_handler"); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(connection, - OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; p->state = omapi_protocol_value_wait; result = omapi_connection_require(connection, vlen); @@ -624,11 +581,8 @@ omapi_protocol_signal_handler(omapi_object_t *h, const char *name, va_list ap) result = omapi_connection_copyout(p->value->u.buffer.value, connection, p->value->u.buffer.len); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(connection, - OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; insert_new_value: if (p->reading_message_values) { @@ -643,22 +597,18 @@ omapi_protocol_signal_handler(omapi_object_t *h, const char *name, va_list ap) */ result = omapi_generic_new(&p->message->object, "omapi_protocol_signal_handler"); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(connection, - OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; } + result = (omapi_set_value ((omapi_object_t *)p->message->object, p->message->id_object, p->name, p->value)); } - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(connection, - OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; + omapi_data_stringdereference(&p->name, "omapi_protocol_signal_handler"); omapi_data_dereference(&p->value, @@ -671,19 +621,14 @@ omapi_protocol_signal_handler(omapi_object_t *h, const char *name, va_list ap) omapi_datatype_data, p->message->authlen); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(connection, - OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; + result = (omapi_connection_copyout (p->message->authenticator->u.buffer.value, connection, p->message->authlen)); - if (result != ISC_R_SUCCESS) { - omapi_connection_disconnect(connection, - OMAPI_FORCE_DISCONNECT); - return (result); - } + if (result != ISC_R_SUCCESS) + goto disconnect; /* XXXTL now do something to verify the signature. */ @@ -699,25 +644,17 @@ omapi_protocol_signal_handler(omapi_object_t *h, const char *name, va_list ap) * XXXDCL duplicated from above. */ if (c->is_client) { - result = isc_mutex_lock(&c->mutex); - if (result != ISC_R_SUCCESS) - goto disconnect; + RUNTIME_CHECK(isc_mutex_lock(&c->mutex) == + ISC_R_SUCCESS); - /* - * This is an unsigned int but on the server it will - * count below 0 for each incoming message received. - * But that's ok, because the server doesn't support - * omapi_connection_wait. - */ + ENSURE(c->messages_expected > 0); c->messages_expected--; - result = isc_condition_signal(&c->waiter); - if (result != ISC_R_SUCCESS) - goto disconnect; + RUNTIME_CHECK(isc_condition_signal(&c->waiter) == + ISC_R_SUCCESS); - result = isc_mutex_unlock(&c->mutex); - if (result != ISC_R_SUCCESS) - goto disconnect; + RUNTIME_CHECK(isc_mutex_unlock(&c->mutex) == + ISC_R_SUCCESS); } /* XXXTL unbind the authenticator. */ @@ -728,13 +665,15 @@ omapi_protocol_signal_handler(omapi_object_t *h, const char *name, va_list ap) OBJECT_DEREF(&p->message, "omapi_protocol_signal_handler"); /* - * Now wait for the next message. + * XXXDCL these gotos could be cleared up with one + * more variable to control a loop around the switch. */ + fprintf(stderr, "going to header_wait, events_pending = %d\n", + c->events_pending); goto to_header_wait; default: - UNEXPECTED_ERROR(__FILE__, __LINE__, - "unknown state in " + UNEXPECTED_ERROR(__FILE__, __LINE__, "unknown state in " "omapi_protocol_signal_handler: %d\n", p->state); @@ -745,6 +684,8 @@ omapi_protocol_signal_handler(omapi_object_t *h, const char *name, va_list ap) /* XXXDCL * 'goto' could be avoided by wrapping the body in another function. + * btw, what happens in C when a file has multiple instances of + * the same label? */ disconnect: omapi_connection_disconnect(connection, OMAPI_FORCE_DISCONNECT); @@ -852,16 +793,27 @@ omapi_protocol_listener_signal(omapi_object_t *h, const char *name, va_list ap) c = va_arg(ap, omapi_object_t *); - INSIST(c != NULL && c->type == omapi_type_connection); + ENSURE(c != NULL && c->type == omapi_type_connection); - obj = isc_mem_get(omapi_mctx, sizeof(*obj)); - if (obj == NULL) - return (ISC_R_NOMEMORY); - memset(obj, 0, sizeof(*obj)); - obj->object_size = sizeof(*obj); - obj->refcnt = 1; - obj->type = omapi_type_protocol; + /* + * Create a new protocol object to oversee the handling of this + * connection. + */ + obj = NULL; + result = omapi_object_new((omapi_object_t **)&obj, omapi_type_protocol, + sizeof(*obj)); + if (result != ISC_R_SUCCESS) + /* + * When the unsuccessful return value is percolated back to + * omapi_listener_accept, then it will remove the only + * reference, which will close and cleanup the connection. + */ + return (result); + /* + * Tie the protocol object bidirectionally to the connection + * object, with the connection as the outer object. + */ OBJECT_REF(&obj->outer, c, "omapi_protocol_accept"); OBJECT_REF(&c->inner, obj, "omapi_protocol_accept"); @@ -873,9 +825,24 @@ omapi_protocol_listener_signal(omapi_object_t *h, const char *name, va_list ap) sizeof(omapi_protocol_header_t)); if (result != ISC_R_SUCCESS) - omapi_connection_disconnect(c, OMAPI_FORCE_DISCONNECT); + /* + * Remove the protocol object's reference to the connection + * object, so that when the unsuccessful return value is + * received in omapi_listener_accept, the connection object + * will be destroyed. + * XXXDCL aigh, this is so confusing. I don't think the + * right thing is being done. + */ + OBJECT_DEREF(&c->inner, "omapi_protocol_accept"); + /* + * Remove one of the references to the object, so it will be + * freed when the connection dereferences its inner object. + * XXXDCL this is what ted did, but i'm not sure my explanation + * is correct. + */ OBJECT_DEREF(&obj, "omapi_protocol_accept"); + return (result); } diff --git a/lib/omapi/support.c b/lib/omapi/support.c index e948a80ed1..b821c267c7 100644 --- a/lib/omapi/support.c +++ b/lib/omapi/support.c @@ -175,6 +175,21 @@ omapi_init(isc_mem_t *mctx) { return (result); } +void +omapi_shutdown() { + omapi_object_type_t *type, *next_type; + + isc_socketmgr_destroy(&omapi_socketmgr); + isc_taskmgr_destroy(&omapi_taskmgr); + isc_timermgr_destroy(&omapi_timermgr); + + for (type = omapi_object_types; type != NULL; type = next_type) { + next_type = type->next; + isc_mem_put(omapi_mctx, type, sizeof(*type)); + } + +} + isc_result_t omapi_object_type_register(omapi_object_type_t **type, const char *name, isc_result_t (*set_value)