Fix some bugs, including an uninitialized-field bug for the QID_FILTER

field, and an ambiguity in the allowable values for the end pointer of
a msg_fifo.  Add a QID_LOCK field, and use it to control access to the
QID_SUBQUEUE field.  Rearrange the locking slightly to make it more
efficient and directed: the top-level qid_lock is now used only to
mediate the QID_ALLOCATEDP and QID_TWIN fields.

diff --git a/v7/src/microcode/os2msg.c b/v7/src/microcode/os2msg.c
index 85ba11276d1e3a47e1de8239ae02092a36510c4f..fe004690a07821752d8e0cfb78678cabe658964e 100644 (file)
--- a/v7/src/microcode/os2msg.c
+++ b/v7/src/microcode/os2msg.c
@@ -1,6 +1,6 @@
 /* -*-C-*-
 
-$Id: os2msg.c,v 1.11 1997/05/11 06:35:05 cph Exp $
+$Id: os2msg.c,v 1.12 1997/05/21 07:32:34 cph Exp $
 
 Copyright (c) 1994-97 Massachusetts Institute of Technology
 
@@ -59,6 +59,7 @@ typedef struct
   qid_receive_filter_t filter; /* filter for received messages */
   tqueue_t * tqueue;           /* thread queue for reception */
   void * subqueue;             /* receiving subqueue */
+  HMTX lock;
 } iqid_t;
 
 static iqid_t queue_array [QID_MAX + 1];
@@ -71,12 +72,10 @@ qid_t OS2_interrupt_qid;
 #define _QID(q) (queue_array [(q)])
 #define QID_ALLOCATEDP(q) ((_QID (q)) . allocatedp)
 #define QID_TWIN(q) ((_QID (q)) . twin)
+#define QID_FILTER(q) ((_QID (q)) . filter)
 #define QID_TQUEUE(q) ((_QID (q)) . tqueue)
 #define QID_SUBQUEUE(q) ((_QID (q)) . subqueue)
-#define QID_FILTER(q) ((_QID (q)) . filter)
-
-#define MSG_QUEUE_TYPE(m) 0
-#define MSG_QUEUE_PRIORITY(m) 0
+#define QID_LOCK(q) ((_QID (q)) . lock)
 
 void
 OS2_initialize_message_queues (void)
@@ -86,9 +85,11 @@ OS2_initialize_message_queues (void)
     while (1)
       {
        (QID_ALLOCATEDP (qid)) = 0;
-       (QID_TQUEUE (qid)) = 0;
        (QID_TWIN (qid)) = QID_NONE;
+       (QID_FILTER (qid)) = 0;
+       (QID_TQUEUE (qid)) = 0;
        (QID_SUBQUEUE (qid)) = 0;
+       (QID_LOCK (qid)) = NULLHANDLE;
        if (qid == QID_MAX)
          break;
        qid += 1;
@@ -130,10 +131,13 @@ allocate_qid (void)
       qid += 1;
     }
   (QID_ALLOCATEDP (qid)) = 1;
-  (QID_TQUEUE (qid)) = 0;
   (QID_TWIN (qid)) = QID_NONE;
-  (QID_SUBQUEUE (qid)) = (OS2_create_msg_fifo ());
   OS2_release_mutex_semaphore (qid_lock);
+  (QID_FILTER (qid)) = 0;
+  (QID_TQUEUE (qid)) = 0;
+  (QID_SUBQUEUE (qid)) = (OS2_create_msg_fifo ());
+  if ((QID_LOCK (qid)) == NULLHANDLE)
+    (QID_LOCK (qid)) = (OS2_create_mutex_semaphore (0, 0));
   return (qid);
 }
 
@@ -156,6 +160,7 @@ OS2_qid_openp (qid_t qid)
 void
 OS2_close_qid (qid_t qid)
 {
+  OS2_request_mutex_semaphore (QID_LOCK (qid));
   while (1)
     {
       msg_t * msg = (OS2_msg_fifo_remove (QID_SUBQUEUE (qid)));
@@ -164,7 +169,10 @@ OS2_close_qid (qid_t qid)
       OS2_destroy_message (msg);
     }
   OS2_destroy_msg_fifo (QID_SUBQUEUE (qid));
+  (QID_FILTER (qid)) = 0;
+  (QID_TQUEUE (qid)) = 0;
   (QID_SUBQUEUE (qid)) = 0;
+  OS2_release_mutex_semaphore (QID_LOCK (qid));
   OS2_request_mutex_semaphore (qid_lock);
   {
     qid_t twin = (QID_TWIN (qid));
@@ -174,7 +182,6 @@ OS2_close_qid (qid_t qid)
        (QID_TWIN (qid)) = QID_NONE;
       }
   }
-  (QID_TQUEUE (qid)) = 0;
   (QID_ALLOCATEDP (qid)) = 0;
   OS2_release_mutex_semaphore (qid_lock);
 }
@@ -318,10 +325,9 @@ OS2_destroy_message (msg_t * message)
 void
 OS2_send_message (qid_t qid, msg_t * message)
 {
-  qid_t twin;
-  OS2_request_mutex_semaphore (qid_lock);
-  twin = (QID_TWIN (qid));
-  if (twin == QID_NONE)
+  qid_t twin = (QID_TWIN (qid));
+  tqueue_t * tqueue;
+  if ((twin == QID_NONE) || ((tqueue = (QID_TQUEUE (twin))) == 0))
     /* Other end of connection has been closed, so discard the
        message.  We used to signal an error here, but this can happen
        pretty easily when closing windows or exiting Scheme.  The only
@@ -331,16 +337,9 @@ OS2_send_message (qid_t qid, msg_t * message)
        generated by the PM thread.  So it's just simpler to ignore
        messages after the receiver decides it's no longer interested
        in them.  */
-    {
-      OS2_release_mutex_semaphore (qid_lock);
-      OS2_destroy_message (message);
-    }
+    OS2_destroy_message (message);
   else
     {
-      tqueue_t * tqueue = (QID_TQUEUE (twin));
-      OS2_release_mutex_semaphore (qid_lock);
-      if (tqueue == 0)
-       OS2_logic_error ("Write to unopened QID.");
       (MSG_SENDER (message)) = twin;
       write_tqueue (tqueue, message);
     }
@@ -435,25 +434,41 @@ write_subqueue (msg_t * message)
       if (message == 0)
        return;
     }
-  OS2_msg_fifo_insert ((QID_SUBQUEUE (qid)), message);
+  OS2_request_mutex_semaphore (QID_LOCK (qid));
+  if (QID_SUBQUEUE (qid))
+    OS2_msg_fifo_insert ((QID_SUBQUEUE (qid)), message);
+  else
+    /* If subqueue is gone, qid has been closed. */
+    OS2_destroy_message (message);
+  OS2_release_mutex_semaphore (QID_LOCK (qid));
 }
 
 static msg_t *
 read_subqueue (qid_t qid)
 {
-  return (OS2_msg_fifo_remove (QID_SUBQUEUE (qid)));
+  msg_t * result;
+  OS2_request_mutex_semaphore (QID_LOCK (qid));
+  result = (OS2_msg_fifo_remove (QID_SUBQUEUE (qid)));
+  OS2_release_mutex_semaphore (QID_LOCK (qid));
+  return (result);
 }
 
 void
 OS2_unread_message (qid_t qid, msg_t * message)
 {
+  OS2_request_mutex_semaphore (QID_LOCK (qid));
   OS2_msg_fifo_insert_front ((QID_SUBQUEUE (qid)), message);
+  OS2_release_mutex_semaphore (QID_LOCK (qid));
 }
 
 static int
 subqueue_emptyp (qid_t qid)
 {
-  return (OS2_msg_fifo_emptyp (QID_SUBQUEUE (qid)));
+  int result;
+  OS2_request_mutex_semaphore (QID_LOCK (qid));
+  result = (OS2_msg_fifo_emptyp (QID_SUBQUEUE (qid)));
+  OS2_release_mutex_semaphore (QID_LOCK (qid));
+  return (result);
 }
 \f
 int
@@ -799,7 +814,7 @@ OS2_destroy_msg_fifo (void * fp)
 
 #define MAYBE_GROW_BUFFER(fifo)                                                \
 {                                                                      \
-  if ((fifo -> count) == (fifo -> buffer_length))                      \
+  if ((fifo -> count) >= (fifo -> buffer_length))                      \
     msg_fifo_grow (fifo);                                              \
 }
 
@@ -810,50 +825,54 @@ OS2_destroy_msg_fifo (void * fp)
     msg_fifo_shrink (fifo);                                            \
 }
 
+#define REALLOC_BUFFER(fifo, new_length)                               \
+{                                                                      \
+  ((fifo) -> buffer_length) = (new_length);                            \
+  ((fifo) -> buffer)                                                   \
+    = (OS_realloc (((fifo) -> buffer),                                 \
+                  (((fifo) -> buffer_length) * (sizeof (void *)))));   \
+}
+
 static void
 msg_fifo_grow (msg_fifo_t * fifo)
 {
-  (fifo -> buffer_length) *= 2;
-  (fifo -> buffer)
-    = (OS_realloc ((fifo -> buffer),
-                  ((fifo -> buffer_length) * (sizeof (void *)))));
-  if ((fifo -> start) > 0)
+  unsigned int old_length = (fifo -> buffer_length);
+  REALLOC_BUFFER (fifo, (old_length * 2));
+  if ((fifo -> end) <= (fifo -> start))
     {
       void ** from = (fifo -> buffer);
-      void ** stop = ((fifo -> buffer) + (fifo -> start));
-      void ** to = ((fifo -> buffer) + (fifo -> count));
+      void ** stop = ((fifo -> buffer) + (fifo -> end));
+      void ** to = ((fifo -> buffer) + old_length);
       while (from < stop)
        (*to++) = (*from++);
-      (fifo -> end) += (fifo -> count);
+      (fifo -> end) += old_length;
     }
 }
 
 static void
 msg_fifo_shrink (msg_fifo_t * fifo)
 {
-  if ((fifo -> start) > (fifo -> end))
+  unsigned int new_length = ((fifo -> buffer_length) / 2);
+  if ((fifo -> end) < (fifo -> start))
     {
       void ** from = ((fifo -> buffer) + (fifo -> start));
       void ** stop = ((fifo -> buffer) + (fifo -> buffer_length));
-      void ** to = (from - ((fifo -> buffer_length) / 2));
+      void ** to = (from - new_length);
       while (from < stop)
        (*to++) = (*from++);
-      (fifo -> start) -= ((fifo -> buffer_length) / 2);
+      (fifo -> start) -= new_length;
     }
-  else if ((fifo -> end) > ((fifo -> buffer_length) / 2))
+  else if ((fifo -> end) > new_length)
     {
       void ** from = ((fifo -> buffer) + (fifo -> start));
       void ** stop = ((fifo -> buffer) + (fifo -> end));
       void ** to = (fifo -> buffer);
       while (from < stop)
        (*to++) = (*from++);
+      (fifo -> end) -= (fifo -> start);
       (fifo -> start) = 0;
-      (fifo -> end) = (fifo -> count);
     }
-  (fifo -> buffer_length) /= 2;
-  (fifo -> buffer)
-    = (OS_realloc ((fifo -> buffer),
-                  ((fifo -> buffer_length) * (sizeof (void *)))));
+  REALLOC_BUFFER (fifo, new_length);
 }
 
 void
@@ -861,11 +880,10 @@ OS2_msg_fifo_insert (void * fp, void * element)
 {
   msg_fifo_t * fifo = fp;
   MAYBE_GROW_BUFFER (fifo);
-  ((fifo -> buffer) [fifo -> end]) = element;
-  (fifo -> end) += 1;
-  (fifo -> count) += 1;
   if ((fifo -> end) == (fifo -> buffer_length))
     (fifo -> end) = 0;
+  ((fifo -> buffer) [(fifo -> end) ++]) = element;
+  (fifo -> count) += 1;
 }
 
 void
@@ -875,8 +893,7 @@ OS2_msg_fifo_insert_front (void * fp, void * element)
   MAYBE_GROW_BUFFER (fifo);
   if ((fifo -> start) == 0)
     (fifo -> start) = (fifo -> buffer_length);
-  (fifo -> start) -= 1;
-  ((fifo -> buffer) [fifo -> start]) = element;
+  ((fifo -> buffer) [-- (fifo -> start)]) = element;
   (fifo -> count) += 1;
 }
 
@@ -887,16 +904,14 @@ OS2_msg_fifo_remove (void * fp)
   void * element;
   if ((fifo -> count) == 0)
     return (0);
-  element = ((fifo -> buffer) [fifo -> start]);
-  (fifo -> start) += 1;
-  (fifo -> count) -= 1;
-  if ((fifo -> count) == 0)
+  element = ((fifo -> buffer) [(fifo -> start) ++]);
+  if ((fifo -> start) == (fifo -> buffer_length))
+    (fifo -> start) = 0;
+  if ((-- (fifo -> count)) == 0)
     {
       (fifo -> start) = 0;
       (fifo -> end) = 0;
     }
-  else if ((fifo -> start) == (fifo -> buffer_length))
-    (fifo -> start) = 0;
   MAYBE_SHRINK_BUFFER (fifo);
   return (element);
 }
@@ -908,12 +923,10 @@ OS2_msg_fifo_remove_last (void * fp)
   void * element;
   if ((fifo -> count) == 0)
     return (0);
+  element = ((fifo -> buffer) [-- (fifo -> end)]);
   if ((fifo -> end) == 0)
     (fifo -> end) = (fifo -> buffer_length);
-  (fifo -> end) -= 1;
-  element = ((fifo -> buffer) [fifo -> end]);
-  (fifo -> count) -= 1;
-  if ((fifo -> count) == 0)
+  if ((-- (fifo -> count)) == 0)
     {
       (fifo -> start) = 0;
       (fifo -> end) = 0;
@@ -951,34 +964,27 @@ OS2_msg_fifo_remove_all (void * fp)
   (fifo -> end) = 0;
   (fifo -> count) = 0;
   if ((fifo -> buffer_length) > BUFFER_MIN_LENGTH)
-    {
-      (fifo -> buffer_length) = (fifo -> buffer_length);
-      (fifo -> buffer)
-       = (OS_realloc ((fifo -> buffer),
-                      ((fifo -> buffer_length) * (sizeof (void *)))));
-    }
+    REALLOC_BUFFER (fifo, BUFFER_MIN_LENGTH);
   return (result);
 }
 
 int
 OS2_msg_fifo_emptyp (void * fp)
 {
-  return ((((msg_fifo_t *) fp) -> count) == 0);
+  msg_fifo_t * fifo = fp;
+  return ((fifo -> count) == 0);
 }
 
 unsigned int
 OS2_msg_fifo_count (void * fp)
 {
-  return (((msg_fifo_t *) fp) -> count);
+  msg_fifo_t * fifo = fp;
+  return (fifo -> count);
 }
 
 void *
 OS2_msg_fifo_last (void * fp)
 {
   msg_fifo_t * fifo = fp;
-  if ((fifo -> count) == 0)
-    return (0);
-  return ((fifo -> buffer)
-         [(((fifo -> end) == 0) ? (fifo -> buffer_length) : (fifo -> end))
-          - 1]);
+  return (((fifo -> count) == 0) ? 0 : ((fifo -> buffer) [(fifo -> end) - 1]));
 }