Changeset 17390

Timestamp:

Apr 8, 2008, 4:15:02 AM (18 years ago)

Author:

eugene

Message:

fix locking / thread conflict problems

File:

• trunk/Ohana/src/opihi/lib.shell/gprint.c (modified) (15 diffs)

trunk/Ohana/src/opihi/lib.shell/gprint.c

@@ -25 +25 @@
      pointer is closed and set to NULL.  setting a new FILE results in the 
      IObuffer being freed and an already opened FILE to be flushed and closed.
+
+
+  gprint & mutex:
+
+     gprintInit : init_stream_mutex
+     gprintGetStream : init_stream_mutex
+     gprintCloseFile : init_stream_mutex
+
+     gprintSetFileAllThreads : set_file_mutex
+     (gprintSetFile -> gprintCloseFile)
+
+     gprintSetFileThisThread : set_file_mutex
+     (gprintGetStream)
+     (gprintSetFile -> gprintCloseFile)
+
+     we are safe from dead locks: init_stream_mutex can be wrapped by set_file_mutex, but
+     not vice-versa
+
 */
 
-static gpStream *streams = NULL;
+static gpStream **streams = NULL;
 static int Nstreams = 0;
 
-static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_mutex_t init_stream_mutex = PTHREAD_MUTEX_INITIALIZER;
 
 void gprintInit () {
@@ -38 +56 @@
 
   /* need to use a mutex to prevent two threads from initing simultaneously */
-  pthread_mutex_lock (&mutex);
-
+  pthread_mutex_lock (&init_stream_mutex);
+
+  // streams is an array of pointers so we can add more streams without changing pointers 
   if (streams == NULL) {
     Nstreams = 2;
-    ALLOCATE (streams, gpStream, Nstreams);
+    ALLOCATE (streams, gpStream *, Nstreams);
   } else {
     Nstreams += 2;
-    REALLOCATE (streams, gpStream, Nstreams);
+    REALLOCATE (streams, gpStream *, Nstreams);
   }
 
@@ -52 +71 @@
 
   N = Nstreams - 2;
-  streams[N].dest = GP_LOG;
-  streams[N].file = stdout;
-  streams[N].name = strcreate ("stdout");
-
-  ALLOCATE (streams[N].buffer, IOBuffer, 1);
-  InitIOBuffer (streams[N].buffer, 64);
-  streams[N].mode = GP_FILE;
-  streams[N].thread = id;
+  ALLOCATE (streams[N], gpStream, 1);
+  streams[N][0].dest = GP_LOG;
+  streams[N][0].file = stdout;
+  streams[N][0].name = strcreate ("stdout");
+
+  ALLOCATE (streams[N][0].buffer, IOBuffer, 1);
+  InitIOBuffer (streams[N][0].buffer, 64);
+  streams[N][0].mode = GP_FILE;
+  streams[N][0].thread = id;
 
   N = Nstreams - 1;
-  streams[N].dest = GP_ERR;
-  streams[N].file = stderr;
-  streams[N].name = strcreate ("stderr");
-
-  ALLOCATE (streams[N].buffer, IOBuffer, 1);
-  InitIOBuffer (streams[N].buffer, 64);
-  streams[N].mode = GP_FILE;
-  streams[N].thread = id;
-
-  pthread_mutex_unlock (&mutex);
+  ALLOCATE (streams[N], gpStream, 1);
+  streams[N][0].dest = GP_ERR;
+  streams[N][0].file = stderr;
+  streams[N][0].name = strcreate ("stderr");
+
+  ALLOCATE (streams[N][0].buffer, IOBuffer, 1);
+  InitIOBuffer (streams[N][0].buffer, 64);
+  streams[N][0].mode = GP_FILE;
+  streams[N][0].thread = id;
+
+  pthread_mutex_unlock (&init_stream_mutex);
 }
 
@@ -78 +99 @@
   int i;
   pthread_t id;
+  gpStream *stream;
+
+  // need to wait for initialization to be finished before getting stream or the array
+  // (streams[i]) may move
+  pthread_mutex_lock (&init_stream_mutex);
 
   id = pthread_self();
@@ -83 +109 @@
   /* find the existing output stream which matches */
   for (i = 0; i < Nstreams; i++) {
-    if (streams[i].dest != dest) continue;
-    if (!pthread_equal (streams[i].thread, id)) continue;
-    return (&streams[i]);
-  }
+    if (streams[i][0].dest != dest) continue;
+    if (!pthread_equal (streams[i][0].thread, id)) continue;
+    stream = streams[i];
+    pthread_mutex_unlock (&init_stream_mutex);
+    return stream;
+  }
+  pthread_mutex_unlock (&init_stream_mutex);
   fprintf (stderr, "programming error: gprintInit not called for thread\n");
   abort ();
@@ -115 +144 @@
   }
 
+  // we cannot do the operation below while another thread is initing
+  pthread_mutex_lock (&init_stream_mutex);
+
   // must we close the existing file? if still being used, then no
   Nmatch = 0;
   for (i = 0; i < Nstreams; i++) {
-    if (stream == &streams[i]) continue;
-    if (streams[i].file == stream[0].file) Nmatch ++;
-  }
+    if (stream == streams[i]) continue;
+    if (streams[i][0].file == stream[0].file) Nmatch ++;
+  }
+  pthread_mutex_unlock (&init_stream_mutex);
+
   if (Nmatch == 0) {
     fflush (stream[0].file);
@@ -130 +164 @@
 }
 
+// this thread only operates on its own stream
 void gprintSetBuffer (gpDest dest) {
 
@@ -145 +180 @@
 }
 
+// this thread only operates on its own stream
 IOBuffer *gprintGetBuffer (gpDest dest) {
 
@@ -158 +194 @@
   int i;
 
-  /* need to use a mutex to prevent two threads from initing simultaneously */
+  // be sure we are not colliding with gprintSetFileThisThread
   pthread_mutex_lock (&set_file_mutex);
 
   for (i = 0; i < Nstreams; i++) {
-    if (streams[i].dest != dest) continue;
-    gprintSetFile (&streams[i], dest, filename);
+    if (streams[i][0].dest != dest) continue;
+    gprintSetFile (streams[i], dest, filename);
   }
 
@@ -170 +206 @@
 }
 
+// this thread only operates on its own stream
 void gprintSetFileThisThread (gpDest dest, char *filename) {
 
   gpStream *stream;
-  stream = gprintGetStream (dest);
-
-  /* need to use a mutex to prevent two threads from initing simultaneously */
+
+  // be sure we are not colliding with gprintSetFileAllThreads
   pthread_mutex_lock (&set_file_mutex);
 
+  stream = gprintGetStream (dest);
   gprintSetFile (stream, dest, filename);
 
@@ -183 +220 @@
   return;
 }
-
 
 void gprintSetFile (gpStream *stream, gpDest dest, char *filename) {
@@ -231 +267 @@
 }
 
+// this thread only operates on its own stream
 FILE *gprintGetFile (gpDest dest) {
 
@@ -238 +275 @@
 }
 
+// this thread only operates on its own stream
 char *gprintGetName (gpDest dest) {
 
@@ -251 +289 @@
   va_list argp;  
 
+  // this thread only writes to its own stream
   stream = gprintGetStream (dest);
 
@@ -273 +312 @@
   gpStream *stream;
 
+  // this thread only writes to its own stream
   stream = gprintGetStream (dest);