From a331edb69f0c8822b28394715f4f6ff7af3fb90e Mon Sep 17 00:00:00 2001 From: Chris Hanson Date: Wed, 21 May 1997 07:32:34 +0000 Subject: [PATCH] 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. --- v7/src/microcode/os2msg.c | 146 ++++++++++++++++++++------------------ 1 file changed, 76 insertions(+), 70 deletions(-) diff --git a/v7/src/microcode/os2msg.c b/v7/src/microcode/os2msg.c index 85ba11276..fe004690a 100644 --- 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); } 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])); } -- 2.25.1