3.0.0rc35: missing pthread_mutex_init()s

Christoph Badura bad at bsd.de
Tue Dec 5 17:18:48 EST 2006


Hey Carter,

while playing around a bit more with the argus clients my system's
pthread implementation carped a bunch about attempts to lock invalid
mutexes. And indeed, the mutexes inside the ArgusHashTables are never
initialized with pthread_mutex_init() as the standard mandates.

I've attached a patch that fixes that.  Maybe it is preferable to
wrap that all in a new ArgusNewHashTable function like the other data
structures.

While being there I wrapped a bunch of pthread_something_or_other() calls
inside #if defined(ARGUS_THREADS) like all the other calls to pthread
functions.

I've also patched the Argus server sources so that the mutexes in the
data structures are initialized, even though the locks are never used.
The pthread related members in the hash struct and model struct seem to
be boiler plate to me, so maybe it is better to remove them altogether.

cheers,
--chris
-------------- next part --------------
--- common/argus_client.c.orig	2006-11-07 05:48:11.000000000 +0100
+++ common/argus_client.c
@@ -3592,6 +3592,9 @@ RaNewBin (struct RaBinProcessStruct *rbp
       ArgusLog (LOG_ERR, "RaMergeQueue: ArgusCalloc error %s\n", strerror(errno));
 
    retn->htable.size = RABINS_HASHTABLESIZE;
+#if defined(ARGUS_THREADS)
+   pthread_mutex_init(&retn->htable.lock, NULL);
+#endif
 
    switch (rbps->nadp.mode) {
       default:
@@ -3998,7 +4001,9 @@ ArgusFindRecord (struct ArgusHashTable *
    struct ArgusHashTableHdr *hashEntry = NULL, *target, *head;
    unsigned int ind = (hstruct->hash % htable->size), i, len;
 
+#if defined(ARGUS_THREADS)
    pthread_mutex_lock(&htable->lock);
+#endif
    if ((target = htable->array[ind]) != NULL) {
       head = target;
       do {
@@ -4030,7 +4035,9 @@ ArgusFindRecord (struct ArgusHashTable *
       }
    }
  
+#if defined(ARGUS_THREADS)
    pthread_mutex_unlock(&htable->lock);
+#endif
 
 #ifdef ARGUSDEBUG
    ArgusDebug (6, "ArgusFindFlow () returning 0x%x\n", retn);
@@ -4047,7 +4054,9 @@ ArgusFindHashEntry (struct ArgusHashTabl
    struct ArgusHashTableHdr *retn = NULL, *target, *head;
    unsigned int ind = (hstruct->hash % htable->size), i, len;
 
+#if defined(ARGUS_THREADS)
    pthread_mutex_lock(&htable->lock);
+#endif
    if ((target = htable->array[ind]) != NULL) {
       head = target;
       do {
@@ -4069,7 +4078,9 @@ ArgusFindHashEntry (struct ArgusHashTabl
             htable->array[ind] = retn;
       }
    }
+#if defined(ARGUS_THREADS)
    pthread_mutex_unlock(&htable->lock);
+#endif
 
 #ifdef ARGUSDEBUG
    ArgusDebug (6, "ArgusFindHashEntry () returning 0x%x\n", retn);
@@ -4114,7 +4125,9 @@ ArgusAddHashEntry (struct ArgusHashTable
 
    ind = (hstruct->hash % table->size);
       
+#if defined(ARGUS_THREADS)
    pthread_mutex_lock(&table->lock);
+#endif
    if ((start = table->array[ind]) != NULL) {
       retn->nxt = start;
       retn->prv = start->prv;
@@ -4124,7 +4137,9 @@ ArgusAddHashEntry (struct ArgusHashTable
       retn->prv = retn->nxt = retn;
 
    table->array[ind] = retn;
+#if defined(ARGUS_THREADS)
    pthread_mutex_unlock(&table->lock);
+#endif
 
    retn->htbl = table;
 
@@ -4142,7 +4157,9 @@ ArgusRemoveHashEntry (struct ArgusHashTa
    struct ArgusHashTable *table = htblhdr->htbl;
    int ind = hash % table->size;
 
+#if defined(ARGUS_THREADS)
    pthread_mutex_lock(&table->lock);
+#endif
    htblhdr->prv->nxt = htblhdr->nxt;
    htblhdr->nxt->prv = htblhdr->prv;
 
@@ -4156,7 +4173,9 @@ ArgusRemoveHashEntry (struct ArgusHashTa
    if (htblhdr->hstruct.buf != NULL)
       ArgusFree (htblhdr->hstruct.buf);
    ArgusFree (htblhdr);
+#if defined(ARGUS_THREADS)
    pthread_mutex_unlock(&table->lock);
+#endif
 
 #ifdef ARGUSDEBUG
    ArgusDebug (6, "ArgusRemoveHashEntry (0x%x) returning\n", htblhdr);
@@ -4169,7 +4188,9 @@ ArgusEmptyHashTable (struct ArgusHashTab
    struct ArgusHashTableHdr *htblhdr = NULL, *tmp;
    int i;
 
+#if defined(ARGUS_THREADS)
    pthread_mutex_lock(&htbl->lock);
+#endif
    for (i = 0; i < htbl->size; i++) {
       if ((htblhdr = htbl->array[i]) != NULL) {
          htblhdr->prv->nxt = NULL;
@@ -4182,7 +4203,9 @@ ArgusEmptyHashTable (struct ArgusHashTab
          htbl->array[i] = NULL;
       }
    }
+#if defined(ARGUS_THREADS)
    pthread_mutex_unlock(&htbl->lock);
+#endif
 
 #ifdef ARGUSDEBUG
    ArgusDebug (6, "ArgusEmptyHashTable (0x%x) returning\n", htbl);
@@ -7234,6 +7257,9 @@ ArgusNewAggregator (struct ArgusParserSt
       ArgusLog (LOG_ERR, "ArgusNewAggregator: ArgusCalloc error %s", strerror(errno));
 
    retn->htable.size = RA_HASHTABLESIZE;
+#if defined(ARGUS_THREADS)
+   pthread_mutex_init(&retn->htable.lock, NULL);
+#endif
 
    retn->RaMetricFetchAlgorithm = ArgusFetchDuration;
 
--- clients/racount.c.orig	2006-11-07 00:48:08.000000000 +0100
+++ clients/racount.c
@@ -136,6 +136,10 @@ ArgusClientInit (struct ArgusParserStruc
       if ((ArgusDstAddrTable.array = (struct ArgusHashTableHdr **)
                   ArgusCalloc (0x10000, sizeof (struct ArgusHashTableHdr))) == NULL)
          ArgusLog (LOG_ERR, "RaTimeInit: ArgusCalloc error %s\n", strerror(errno));
+#if defined(ARGUS_THREADS)
+      pthread_mutex_init(&ArgusSrcAddrTable.lock, NULL);
+      pthread_mutex_init(&ArgusDstAddrTable.lock, NULL);
+#endif
 
       parser->RaInitialized++;
    }

--- ratop/ramatrix.c.orig	2006-11-17 18:14:17.000000000 +0100
+++ ratop/ramatrix.c
@@ -3336,6 +3336,9 @@ RaTopNewProcess(struct ArgusParserStruct
          ArgusLog (LOG_ERR, "RaTopNewProcess: ArgusCalloc error %s\n", strerror(errno));
       } else {
          retn->htable.size = 0x100000;
+#if defined(ARGUS_THREADS)
+         pthread_mutex_init(&retn->htable.lock, NULL);
+#endif
       }
 
       if ((RaMatrixDisplay = ArgusNewQueue()) == NULL)

--- ratop/ratop.c.orig	2006-11-10 17:38:39.000000000 +0100
+++ ratop/ratop.c
@@ -3156,6 +3156,9 @@ RaTopNewProcess(struct ArgusParserStruct
          ArgusLog (LOG_ERR, "RaTopNewProcess: ArgusCalloc error %s\n", strerror(errno));
       } else {
          retn->htable.size = 0x100000;
+#if defined(ARGUS_THREADS)
+         pthread_mutex_init(&retn->htable.lock, NULL);
+#endif
       }
  
    } else
-------------- next part --------------
--- argus/ArgusOutput.c.orig	2006-10-16 21:57:15.000000000 +0200
+++ argus/ArgusOutput.c
@@ -75,6 +75,9 @@ ArgusNewOutput (struct ArgusSourceStruct
 
    retn->ArgusSrc   = src;
    retn->ArgusModel = model;
+#if defined(ARGUS_THREADS)
+   pthread_mutex_init(&retn->lock, NULL);
+#endif
 
 #ifdef ARGUSDEBUG
    ArgusDebug (1, "ArgusNewOutput() returning retn 0x%x\n", retn);

--- argus/ArgusModeler.c.orig	2006-11-17 04:31:59.000000000 +0100
+++ argus/ArgusModeler.c
@@ -83,6 +83,10 @@ ArgusNewModeler()
 
    retn->ArgusHashTable->size = ARGUS_HASHTABLESIZE;
 
+#if defined(ARGUS_THREADS)
+   pthread_mutex_init(&retn->ArgusHashTable->lock, NULL);
+#endif
+
    if ((retn->hstruct = (struct ArgusHashStruct *) ArgusCalloc (1, sizeof (struct ArgusHashStruct))) == NULL)
       ArgusLog (LOG_ERR, "ArgusNewModeler () ArgusCalloc error %s\n", strerror(errno));
 
@@ -97,6 +101,10 @@ ArgusNewModeler()
 
    gettimeofday (&retn->ArgusGlobalTime, 0L);
    retn->ArgusThisLLC = &ArgusThisLLCBuffer;
+
+#if defined(ARGUS_THREADS)
+   pthread_mutex_init(&retn->lock, NULL);
+#endif
  
 #ifdef ARGUSDEBUG
    ArgusDebug (1, "ArgusNewModeler() returning 0x%x\n", retn);


More information about the argus mailing list