From 38aac4c8688cf18df3495c1131b9e91051e941a6 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 3 May 2018 09:16:08 -0400 Subject: [PATCH] Eliminate message priority from TR_MQ / TR_MQ_MSG This was an unnecessary feature that had caused several bugs, most recently #80. Rather than debug that, this removes the priorities, returning to a simple queue. --- common/tests/mq_test.c | 8 ++++---- common/tests/thread_test.c | 2 +- common/tr_mq.c | 42 +++++------------------------------------- include/tr_mq.h | 14 +------------- tr/tr_tid.c | 4 ++-- tr/tr_trp.c | 12 ++++++------ 6 files changed, 19 insertions(+), 63 deletions(-) diff --git a/common/tests/mq_test.c b/common/tests/mq_test.c index 5c04b37..b309e25 100644 --- a/common/tests/mq_test.c +++ b/common/tests/mq_test.c @@ -59,7 +59,7 @@ int main(void) mq->notify_cb=notify_cb; mq->notify_cb_arg=mq_name; - msg1=tr_mq_msg_new(NULL,"Message 1", TR_MQ_PRIO_NORMAL); + msg1= tr_mq_msg_new(NULL, "Message 1"); assert(asprintf((char **)&(msg1->p), "First message.\n")!=-1); msg1->p_free=free; tr_mq_add(mq, msg1); @@ -67,7 +67,7 @@ int main(void) assert(mq->tail==msg1); assert(msg1->next==NULL); - msg2=tr_mq_msg_new(NULL, "Message 2", TR_MQ_PRIO_NORMAL); + msg2= tr_mq_msg_new(NULL, "Message 2"); assert(asprintf((char **)&(msg2->p), "Second message.\n")!=-1); msg2->p_free=free; tr_mq_add(mq, msg2); @@ -87,7 +87,7 @@ int main(void) } else printf("no message to pop\n"); - msg3=tr_mq_msg_new(NULL, "Message 3", TR_MQ_PRIO_NORMAL); + msg3= tr_mq_msg_new(NULL, "Message 3"); assert(asprintf((char **)&(msg3->p), "%s", "Third message.\n")!=-1); msg3->p_free=free; tr_mq_add(mq, msg3); @@ -127,7 +127,7 @@ int main(void) } else printf("no message to pop\n"); - msg4=tr_mq_msg_new(NULL, "Message 4", TR_MQ_PRIO_NORMAL); + msg4= tr_mq_msg_new(NULL, "Message 4"); assert(asprintf((char **)&(msg4->p), "%s", "Fourth message.\n")!=-1); msg4->p_free=free; tr_mq_add(mq, msg4); diff --git a/common/tests/thread_test.c b/common/tests/thread_test.c index 14aac6f..35d7882 100644 --- a/common/tests/thread_test.c +++ b/common/tests/thread_test.c @@ -53,7 +53,7 @@ struct thread_data { static TR_MQ_MSG *make_msg(char *label, int n) { TR_MQ_MSG *msg=NULL; - msg=tr_mq_msg_new(NULL, "Message", TR_MQ_PRIO_NORMAL); + msg= tr_mq_msg_new(NULL, "Message"); assert(-1!=asprintf((char **)&(msg->p), "%s: %d messages to go...", label, n)); msg->p_free=free; return msg; diff --git a/common/tr_mq.c b/common/tr_mq.c index 07080b6..1ff2cbf 100644 --- a/common/tr_mq.c +++ b/common/tr_mq.c @@ -49,12 +49,11 @@ static int tr_mq_msg_destructor(void *object) return 0; } -TR_MQ_MSG *tr_mq_msg_new(TALLOC_CTX *mem_ctx, const char *message, TR_MQ_PRIORITY prio) +TR_MQ_MSG *tr_mq_msg_new(TALLOC_CTX *mem_ctx, const char *message) { TR_MQ_MSG *msg=talloc(mem_ctx, TR_MQ_MSG); if (msg!=NULL) { msg->next=NULL; - msg->prio=prio; msg->message=talloc_strdup(msg, message); if (msg->message==NULL) { talloc_free(msg); @@ -72,11 +71,6 @@ void tr_mq_msg_free(TR_MQ_MSG *msg) talloc_free(msg); } -TR_MQ_PRIORITY tr_mq_msg_get_prio(TR_MQ_MSG *msg) -{ - return msg->prio; -} - const char *tr_mq_msg_get_message(TR_MQ_MSG *msg) { return msg->message; @@ -121,7 +115,6 @@ TR_MQ *tr_mq_new(TALLOC_CTX *mem_ctx) mq->head=NULL; mq->tail=NULL; - mq->last_hi_prio=NULL; mq->notify_cb=NULL; mq->notify_cb_arg=NULL; @@ -208,22 +201,6 @@ static void tr_mq_append(TR_MQ *mq, TR_MQ_MSG *msg) talloc_steal(mq, msg); } -static void tr_mq_append_high_prio(TR_MQ *mq, TR_MQ_MSG *new) -{ - if (tr_mq_get_head(mq)==NULL) { - tr_mq_set_head(mq, new); - tr_mq_set_tail(mq, new); - } else if (mq->last_hi_prio==NULL) { - tr_mq_msg_set_next(new, tr_mq_get_head(mq)); /* add to front of list */ - tr_mq_set_head(mq, new); /* update head of list */ - } else { - tr_mq_msg_set_next(new, tr_mq_msg_get_next(mq->last_hi_prio)); - tr_mq_msg_set_next(mq->last_hi_prio, new); /* add to end of hi prio msgs */ - } - mq->last_hi_prio=new; /* in any case, this is now the last high priority msg */ - talloc_steal(mq,new); -} - #define DEBUG_TR_MQ 0 #if DEBUG_TR_MQ static void tr_mq_print(TR_MQ *mq) @@ -234,8 +211,8 @@ static void tr_mq_print(TR_MQ *mq) tr_debug("tr_mq_print: mq contents:"); while(m!=NULL) { ii++; - tr_debug("tr_mq_print: Entry %02d: %-15s (prio %d)", - ii, tr_mq_msg_get_message(m), tr_mq_msg_get_prio(m)); + tr_debug("tr_mq_print: Entry %02d: %-15s", + ii, tr_mq_msg_get_message(m)); m=tr_mq_msg_get_next(m); } } @@ -249,14 +226,8 @@ void tr_mq_add(TR_MQ *mq, TR_MQ_MSG *msg) tr_mq_lock(mq); was_empty=tr_mq_empty(mq); - switch (tr_mq_msg_get_prio(msg)) { - case TR_MQ_PRIO_HIGH: - tr_mq_append_high_prio(mq, msg); - break; - default: - tr_mq_append(mq, msg); - break; - } + tr_mq_append(mq, msg); + /* before releasing the mutex, get notify_cb data out of mq */ notify_cb=mq->notify_cb; notify_cb_arg=mq->notify_cb_arg; @@ -320,9 +291,6 @@ TR_MQ_MSG *tr_mq_pop(TR_MQ *mq, struct timespec *ts_abort) popped=tr_mq_get_head(mq); tr_mq_set_head(mq, tr_mq_msg_get_next(popped)); /* popped is the old head */ - if (popped==mq->last_hi_prio) - mq->last_hi_prio=NULL; - if (tr_mq_get_head(mq)==NULL) tr_mq_set_tail(mq, NULL); /* just popped the last element */ } diff --git a/include/tr_mq.h b/include/tr_mq.h index f60c7af..b15081c 100644 --- a/include/tr_mq.h +++ b/include/tr_mq.h @@ -39,20 +39,10 @@ #include #include -/* Note on mq priorities: High priority messages are guaranteed to be - * processed before any normal priority messages. Otherwise, messages - * will be processed in the order they are added to the queue. */ - -typedef enum tr_mq_priority { - TR_MQ_PRIO_NORMAL=0, - TR_MQ_PRIO_HIGH -} TR_MQ_PRIORITY; - /* msg for inter-thread messaging */ typedef struct tr_mq_msg TR_MQ_MSG; struct tr_mq_msg { TR_MQ_MSG *next; - TR_MQ_PRIORITY prio; char *message; void *p; /* payload */ void (*p_free)(void *); /* function to free payload */ @@ -67,7 +57,6 @@ struct tr_mq { pthread_cond_t have_msg_cond; TR_MQ_MSG *head; TR_MQ_MSG *tail; - TR_MQ_MSG *last_hi_prio; TR_MQ_NOTIFY_FN notify_cb; /* callback when queue becomes non-empty */ void *notify_cb_arg; }; @@ -75,9 +64,8 @@ struct tr_mq { /* message string for sending trpc messages */ #define TR_MQMSG_TRPC_SEND "trpc send msg" -TR_MQ_MSG *tr_mq_msg_new(TALLOC_CTX *mem_ctx, const char *msg, TR_MQ_PRIORITY prio); +TR_MQ_MSG *tr_mq_msg_new(TALLOC_CTX *mem_ctx, const char *msg); void tr_mq_msg_free(TR_MQ_MSG *msg); -TR_MQ_PRIORITY tr_mq_msg_get_prio(TR_MQ_MSG *msg); const char *tr_mq_msg_get_message(TR_MQ_MSG *msg); void *tr_mq_msg_get_payload(TR_MQ_MSG *msg); void tr_mq_msg_set_payload(TR_MQ_MSG *msg, void *p, void (*p_free)(void *)); diff --git a/tr/tr_tid.c b/tr/tr_tid.c index 038cc3c..fc55bc3 100644 --- a/tr/tr_tid.c +++ b/tr/tr_tid.c @@ -196,9 +196,9 @@ cleanup: /* mq is still valid, so we can queue our response */ tr_debug("tr_tids_req_fwd_thread: thread %d using valid msg queue.", cookie->thread_id); if (success) - msg=tr_mq_msg_new(tmp_ctx, TR_TID_MQMSG_SUCCESS, TR_MQ_PRIO_NORMAL); + msg= tr_mq_msg_new(tmp_ctx, TR_TID_MQMSG_SUCCESS); else - msg=tr_mq_msg_new(tmp_ctx, TR_TID_MQMSG_FAILURE, TR_MQ_PRIO_NORMAL); + msg= tr_mq_msg_new(tmp_ctx, TR_TID_MQMSG_FAILURE); if (msg==NULL) tr_notice("tr_tids_req_fwd_thread: thread %d unable to allocate response msg.", cookie->thread_id); diff --git a/tr/tr_trp.c b/tr/tr_trp.c index da92be4..44f001a 100644 --- a/tr/tr_trp.c +++ b/tr/tr_trp.c @@ -93,7 +93,7 @@ static TRP_RC tr_trps_msg_handler(TRPS_INSTANCE *trps, /* n.b., conn is available here, but do not hold onto the reference * because it may be cleaned up if the originating connection goes * down before the message is processed */ - mq_msg=tr_mq_msg_new(tmp_ctx, TR_MQMSG_MSG_RECEIVED, TR_MQ_PRIO_NORMAL); + mq_msg= tr_mq_msg_new(tmp_ctx, TR_MQMSG_MSG_RECEIVED); if (mq_msg==NULL) { return TRP_NOMEM; } @@ -147,7 +147,7 @@ static void *tr_trps_thread(void *arg) if (trps_authorize_connection(trps, conn)!=TRP_SUCCESS) goto cleanup; - msg=tr_mq_msg_new(tmp_ctx, TR_MQMSG_TRPS_CONNECTED, TR_MQ_PRIO_HIGH); + msg= tr_mq_msg_new(tmp_ctx, TR_MQMSG_TRPS_CONNECTED); tr_mq_msg_set_payload(msg, (void *)tr_dup_name(trp_connection_get_peer(conn)), tr_free_name_helper); if (msg==NULL) { tr_err("tr_trps_thread: error allocating TR_MQ_MSG"); @@ -159,7 +159,7 @@ static void *tr_trps_thread(void *arg) trps_handle_connection(trps, conn); cleanup: - msg=tr_mq_msg_new(tmp_ctx, TR_MQMSG_TRPS_DISCONNECTED, TR_MQ_PRIO_HIGH); + msg= tr_mq_msg_new(tmp_ctx, TR_MQMSG_TRPS_DISCONNECTED); tr_mq_msg_set_payload(msg, (void *)conn, NULL); /* do not pass a free routine */ if (msg==NULL) tr_err("tr_trps_thread: error allocating TR_MQ_MSG"); @@ -226,7 +226,7 @@ static void tr_trps_cleanup_trpc(TRPS_INSTANCE *trps, TRPC_INSTANCE *trpc) TR_MQ_MSG *msg; /* tell the trpc thread to exit */ - msg = tr_mq_msg_new(NULL, TR_MQMSG_TRPC_EXIT_OK, TR_MQ_PRIO_NORMAL); + msg = tr_mq_msg_new(NULL, TR_MQMSG_TRPC_EXIT_OK); if (msg) { tr_debug("tr_trps_cleanup_trpc: Notifying thread that it may now exit"); trpc_mq_add(trpc, msg); @@ -680,7 +680,7 @@ static void *tr_trpc_thread(void *arg) } tr_debug("tr_trpc_thread: connected to peer %s", peer_gssname->buf); - msg=tr_mq_msg_new(tmp_ctx, TR_MQMSG_TRPC_CONNECTED, TR_MQ_PRIO_HIGH); + msg= tr_mq_msg_new(tmp_ctx, TR_MQMSG_TRPC_CONNECTED); tr_mq_msg_set_payload(msg, (void *)tr_dup_name(peer_gssname), tr_free_name_helper); if (msg==NULL) { tr_err("tr_trpc_thread: error allocating TR_MQ_MSG"); @@ -737,7 +737,7 @@ static void *tr_trpc_thread(void *arg) trpc->shutting_down = 1; /* Send a DISCONNECTED message to the main thread */ - msg=tr_mq_msg_new(tmp_ctx, TR_MQMSG_TRPC_DISCONNECTED, TR_MQ_PRIO_NORMAL); + msg= tr_mq_msg_new(tmp_ctx, TR_MQMSG_TRPC_DISCONNECTED); tr_mq_msg_set_payload(msg, (void *)trpc, NULL); /* do not pass a free routine */ if (msg==NULL) { /* can't notify main thread of exit - just do it and hope for the best */ -- 2.1.4