Eliminate message priority from TR_MQ / TR_MQ_MSG
authorJennifer Richards <jennifer@painless-security.com>
Thu, 3 May 2018 13:16:08 +0000 (09:16 -0400)
committerJennifer Richards <jennifer@painless-security.com>
Thu, 3 May 2018 13:16:08 +0000 (09:16 -0400)
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
common/tests/thread_test.c
common/tr_mq.c
include/tr_mq.h
tr/tr_tid.c
tr/tr_trp.c

index 5c04b37..b309e25 100644 (file)
@@ -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);
index 14aac6f..35d7882 100644 (file)
@@ -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;
index 07080b6..1ff2cbf 100644 (file)
@@ -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 */
   }
index f60c7af..b15081c 100644 (file)
 #include <pthread.h>
 #include <time.h>
 
-/* 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 *));
index 038cc3c..fc55bc3 100644 (file)
@@ -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);
index da92be4..44f001a 100644 (file)
@@ -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 */